Room. Incorrect behavior of methods with @Upsert annotation.

[AndreyPatrin]

Description

In Room EntityUpsertionAdapter we can see code:

    private const val ErrorCode = "1555"

    fun upsert(entity: T) {
        try {
            insertionAdapter.insert(entity)
        } catch (ex: SQLiteConstraintException) {
            checkUniquenessException(ex)
            updateAdapter.handle(entity)
        }
    }

    private fun checkUniquenessException(ex: SQLiteConstraintException) {
        val message = ex.message ?: throw ex
        if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.KITKAT) {
            if (!message.contains(ErrorMsg, ignoreCase = true)) {
                throw ex
            }
        } else {
            if (!message.contains(ErrorCode, ignoreCase = true)) {
                throw ex
            }
        }
    }

When code is executed on real device the message variable will be something like this "UNIQUE constraint failed: MeEntity.id (code 1555 SQLITE_CONSTRAINT_PRIMARYKEY[1555])"
When code is executed on Robolectric the message variable will be equal "Cannot execute for last inserted row ID"

As result:

• on real device updateAdapter.handle(entity) will be executed
• on Robolectric exception will be thrown

Steps to Reproduce

@Entity(
        primaryKeys = ["id"]
)
data class MyEntity(
        val id: Long,
        val name: String
)

@Dao
abstract class MyDao {
    @Upsert
    abstract override suspend fun upsertMyEntity(data: MyEntity)
}

val data = MyEntity(10L, "name")
db.upsertMyEntity(data)
db.upsertMyEntity(data)     // at this point db throws exception.

Robolectric & Android Version

robolectric:4.10.3
android: 12
room: 2.5.1

Link to a public git repo demonstrating the problem:

[BekhtaTaras]

This happens for me as well, but in my case the test fails only when running it on Windows machine. It passes for my colleagues running the exact same test on MacOS.
It also passes during the GitHub CI run using ubuntu-latest runner.

[BekhtaTaras]

I don't why does it differ between different operating systems but one possible solution (from the robolectric team) can be to not completely replace the exception messages with their own one.
For example in the ShadowLegacySQLiteConnection there is a function private static <T> T getFuture(final String comment, final Future<T> future) that catches SQLiteException and re-throws it as a RuntimeException with updated message.
It needs one line update to something like this:

final RuntimeException sqlException =
              getSqliteException("Cannot " + comment. + ((SQLiteException) t).getMessage(), ((SQLiteException) t).getBaseErrorCode());

So the updated exception message still contains the original message as well (with the proper error code and unique word in it).

[hoisie]

Hi @BekhtaTaras,

Unfortunately, on Windows, the Android SQLite database is powered by a legacy version of sqlite4java, and on Mac+Linux it is using real native Android SQLite code.

I wish Windows had feature parity, but unfortunately we haven't been able to get that prioritized due to other projects and initiatives. Similarly, none of the current Robolectric maintainers use Windows. It is technically possible to support Android SQLite in Windows, it just requires a potentially non-trivial amount of work and investigation.

If you (or someone else) would like to contribute to Windows support, we'd be happy to provide assistance and guidance.

[ampeixoto]

I'm having the exact same issue, on windows...

@hoisie I understand... Would it be too bad to implement the fix/workaround proposed by @BekhtaTaras ?

For example in the ShadowLegacySQLiteConnection there is a function private static <T> T getFuture(final String comment, final Future<T> future) that catches SQLiteException and re-throws it as a RuntimeException with updated message.
It needs one line update to something like this:
`final RuntimeException sqlException =
              getSqliteException("Cannot " + comment. + ((SQLiteException) t).getMessage(), ((SQLiteException) t).getBaseErrorCode());`

Or it would bring other unwanted side effects?

[hoisie]

Making error messages consistent sounds good to me. A lot of these exceptions are generated from the Android SQLite C++ bindings, which are not run on Windows (they use the sqlite4java swig bindings). However, we can probably just shim the exceptions in Java code. If libraries like room check for specific Android SQLite exceptions, we should probably make them consistent.

[ampeixoto]

Making error messages consistent sounds good to me. A lot of these exceptions are generated from the Android SQLite C++ bindings, which are not run on Windows (they use the sqlite4java swig bindings). However, we can probably just shim the exceptions in Java code. If libraries like room check for specific Android SQLite exceptions, we should probably make them consistent.

Thanks @hoisie ! So, you think we could expect some sort of fix on an upcoming release?

[hoisie]

@ampeixoto I'll add this issue to my queue, but I cannot really guarantee anything. If this issue is important to you, I would definitely recommend sending a PR to fix it, and I would be happy to review it.

[hoisie]

This should be fixed in the latest 4.12-SNAPSHOT, I would give that a try.

[BekhtaTaras]

Thank you @hoisie I will try that later today

[BekhtaTaras]

I have just tried that and now all of my tests pass. Thanks again @hoisie 🥳