greenrobot / greenDAO

greenDAO is a light & fast ORM solution for Android that maps objects to SQLite databases.
http://greenrobot.org/greendao/
12.63k stars 2.89k forks source link

Throw exception when null value in WhereCondition #1031

Open jeffdgr8 opened 4 years ago

jeffdgr8 commented 4 years ago

It's possible to get an IllegalArgumentException all the way down at the database level if a query was built using null values in any of the WhereConditions. The stack trace looks like this:

Fatal Exception: java.lang.IllegalArgumentException
the bind value at index 1 is null
    net.sqlcipher.database.SQLiteProgram.bindString (SQLiteProgram.java:237)
    net.sqlcipher.database.SQLiteQuery.bindString (SQLiteQuery.java:185)
    net.sqlcipher.database.SQLiteDirectCursorDriver.query (SQLiteDirectCursorDriver.java:48)
    net.sqlcipher.database.SQLiteDatabase.rawQueryWithFactory (SQLiteDatabase.java:1772)
    net.sqlcipher.database.SQLiteDatabase.rawQuery (SQLiteDatabase.java:1737)
    de.greenrobot.dao.database.EncryptedDatabase.rawQuery (EncryptedDatabase.java:31)
    de.greenrobot.dao.query.Query.list (Query.java:76)
    ...

It is difficult to debug the source of the poorly constructed queries, that didn't check for null before creating the WhereConditions, especially when the query was built using an AsyncSession. In that case the stack trace looks like this:

Fatal Exception: de.greenrobot.dao.async.AsyncDaoException
    de.greenrobot.dao.async.AsyncOperation.getResult (AsyncOperation.java:105)
    com.my.code.MyClass.onAsyncOperationCompleted (MyClass.java:42)
    de.greenrobot.dao.async.AsyncOperationExecutor.handleOperationCompleted (AsyncOperationExecutor.java:241)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperationAndPostCompleted (AsyncOperationExecutor.java:260)
    de.greenrobot.dao.async.AsyncOperationExecutor.run (AsyncOperationExecutor.java:167)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1080)
    java.lang.Thread.run (Thread.java:841)
Caused by java.lang.IllegalArgumentException
    net.sqlcipher.database.SQLiteProgram.bindString (SQLiteProgram.java:237)
    net.sqlcipher.database.SQLiteQuery.bindString (SQLiteQuery.java:185)
    net.sqlcipher.database.SQLiteDirectCursorDriver.query (SQLiteDirectCursorDriver.java:48)
    net.sqlcipher.database.SQLiteDatabase.rawQueryWithFactory (SQLiteDatabase.java:1758)
    net.sqlcipher.database.SQLiteDatabase.rawQuery (SQLiteDatabase.java:1723)
    de.greenrobot.dao.database.EncryptedDatabase.rawQuery (EncryptedDatabase.java:31)
    de.greenrobot.dao.query.Query.list (Query.java:76)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperation (AsyncOperationExecutor.java:311)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperationAndPostCompleted (AsyncOperationExecutor.java:259)
    de.greenrobot.dao.async.AsyncOperationExecutor.run (AsyncOperationExecutor.java:167)
    ...

In this case, the IllegalArgumentException does not even log the actual message, at least indicating the index where the null value was found. But even knowing the index, it's much more difficult to figure out which was the specific WhereCondition that bound the value to that index.

It's much more helpful if the WhereCondition itself throws an exception at the time of creation in order to detect the invalid value sooner. The PropertyCondition.checkValueForType() method previously would throw a DaoException in the case of a Date or boolean property type being null. But every other type did not have any explicit checking. This change will throw a DaoException if any null value is found.