Re: [VOTE] Release of DbUtils 1.2 RC1
sebb wrote: I think there are some problems with thread-safety. Yikes! I didn't investigate very carefully the thread-safety claims of these classes (again, not much has changed since 1.1) but I agree with your assessment, now that I open my eyes and think about it. In revision 752369 I incorporated the patches you attached to http://issues.apache.org/jira/browse/DBUTILS-51 However, the KeyedHandler instance variables are protected, rather than private, so making them final would perhaps break some programs. I don't think the class can be thread-safe at present. I don't think there's a lot of risk that users required these values to be mutable; I would assume that most users just called the super constructor, so I applied this patch, too. -Dan - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC1
I think there are some problems with thread-safety. QueryRunner Javadoc says the class is thread-safe. However it has a protected mutable variable DataSource which can also be set/got via public methods. If one thread sets the variable, another may not see the correct value, so the class is not thread-safe. ArrayHandler has a non-final instance variable "convert" which could be made final. Likewise BasicRowProcessor. This would make the classes immutable, and therefore thread-safe. It's not clear if they are thread-safe without making the variables final. The same applies to most of the handlers - they have instance variables which could be made final. However, the KeyedHandler instance variables are protected, rather than private, so making them final would perhaps break some programs. I don't think the class can be thread-safe at present. I'll raise a JIRA with suggested patches. On 11/03/2009, Dan Fabulich wrote: > sebb wrote: > > > > -0.5 because the unit tests seem wrong. > > > > I've incorporated your feedback in revision 752322. For the record, all of > those tests predate DbUtils 1.1; we haven't touched them in years. > > I think these test changes are pretty minor; not worth an additional RC. > What do you think? > > -Dan > > > - > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC1
Jörg Schaible wrote: However, IBM JDK 6 fails here: --- Test set: org.apache.commons.dbutils.QueryRunnerTest --- Tests run: 8, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.341 sec <<< FAILURE! testFillStatementWithBeanErrorReadMethodPrivate(org.apache.commons.dbutils.QueryRunnerTest) It seems that this JDK does more internal checks. Possibly fixed in trunk revision 752329. (Fumbled fingers and accidentally forgot to add a commit message.) Please try again in trunk; I think it should work. -Dan - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC1
sebb wrote: -0.5 because the unit tests seem wrong. I've incorporated your feedback in revision 752322. For the record, all of those tests predate DbUtils 1.1; we haven't touched them in years. I think these test changes are pretty minor; not worth an additional RC. What do you think? -Dan - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC1
On 08/03/2009, Dan Fabulich wrote: > > My first attempt at releasing a commons project; please be gentle. :-) > > Compatibility warning: This version is mostly a bugfix release, but to fix > DBUTILS-31 we had to upgrade the JVM dependency from JDK 1.3 to JDK 1.4. > Except for that, it is backwards compatible with DbUtils 1.1. > > PLEASE TEST THIS RELEASE WITH A REAL DATABASE! > > Although this project has reasonable unit tests, it has no integration > tests with any actual databases; it is quite possible that the fix for > DBUTILS-31 has broken something on Oracle, MS SQL Server, Derby, or your > favorite database. The Unit test ResultSetIteratorTest looks rather odd, in that it assigns and checks the row[] array to be non-null within the iterator loop, but checks row[0] etc outside the loop. Is it really intended to only check the last row[] array? If so, it ought to check for null first. Similar comments apply to: ArrayListHandlerTest BeanListHandlerTest MapListHandlerTest BasicRowProcessorTest BeanProcessorTest Also, ProxyFactoryTest uses "instanceof" checks against methods that are declared to return the class being tested - these should surely check for null instead? > To verify DBUTILS-31, use QueryRunner to put a null value in a field, e.g. > with QueryRunner.update. Ideally it would be good to verify putting nulls > in fields of various types: char, varchar, int, boolean, date, etc. > > -- > > Tag: > > https://svn.apache.org/repos/asf/commons/proper/dbutils/tags/DBUTILS_1_2 DOAP ought to have an AL header. > Site: > > http://people.apache.org/builds/commons/dbutils/1.2/RC1/site/index.html > "...and relies only on a standard Java 1.3 or later JRE. " That should be 1.4. Given that it is now using 1.4, Junit could be updated to 3.8.2 > Binaries: > > http://people.apache.org/builds/commons/dbutils/1.2/RC1/staged/commons-dbutils/commons-dbutils/1.2/ Sigs and hashes OK N&L files look OK. Source archive agrees with SVN tag. > [ ] +1 release it > [ ] +0 go ahead I don't care > [ ] -1 no, do not release it because -0.5 because the unit tests seem wrong. > > > > - > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC1
Hi Dan, I've downloaded the src tar ball and tried to build the version with my compiler zoo. It runs fine with all Sun JDKs, IcedTea6, Blackdown 1.4.2, JRockit 1.4 and 1.5. As usual the build faisl for IBM JDK 1.4 and 1.5 because Maven fails itself on this versions. However, IBM JDK 6 fails here: --- Test set: org.apache.commons.dbutils.QueryRunnerTest --- Tests run: 8, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.341 sec <<< FAILURE! testFillStatementWithBeanErrorReadMethodPrivate(org.apache.commons.dbutils.QueryRunnerTest) Time elapsed: 0.084 sec <<< ERROR! java.beans.IntrospectionException: Modifier for getter method should be public. at java.beans.PropertyDescriptor.setReadMethod(PropertyDescriptor.java:123) at java.beans.PropertyDescriptor.(PropertyDescriptor.java:76) at org.apache.commons.dbutils.QueryRunnerTest.testFillStatementWithBeanErrorReadMethodPrivate(QueryRunnerTest.java:157) It seems that this JDK does more internal checks. This is not a show stopper for the release though, since it does not affect the functionality of the DBUtils itself, but if you have to cut another RC, it would be good if the test can also pass on this JDK. So, in general: +1 - Jörg - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[VOTE] Release of DbUtils 1.2 RC1
My first attempt at releasing a commons project; please be gentle. :-) Compatibility warning: This version is mostly a bugfix release, but to fix DBUTILS-31 we had to upgrade the JVM dependency from JDK 1.3 to JDK 1.4. Except for that, it is backwards compatible with DbUtils 1.1. PLEASE TEST THIS RELEASE WITH A REAL DATABASE! Although this project has reasonable unit tests, it has no integration tests with any actual databases; it is quite possible that the fix for DBUTILS-31 has broken something on Oracle, MS SQL Server, Derby, or your favorite database. To verify DBUTILS-31, use QueryRunner to put a null value in a field, e.g. with QueryRunner.update. Ideally it would be good to verify putting nulls in fields of various types: char, varchar, int, boolean, date, etc. -- Tag: https://svn.apache.org/repos/asf/commons/proper/dbutils/tags/DBUTILS_1_2 Site: http://people.apache.org/builds/commons/dbutils/1.2/RC1/site/index.html Binaries: http://people.apache.org/builds/commons/dbutils/1.2/RC1/staged/commons-dbutils/commons-dbutils/1.2/ [ ] +1 release it [ ] +0 go ahead I don't care [ ] -1 no, do not release it because - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org