Add support for transaction rollbacks

[jacek-marchwicki]

Hi, first of all, thanks for the great library! It made my life much easier! 🎉

Previously, when an exception was thrown during a transaction, the changes were still applied to the database, even if the transaction was intended to be aborted.

For example, consider this code:

database.writeTransaction("users") {
    objectStore("users").add(
        jso {
            id = "7740f7c4-f889-498a-bc6d-f88dabdcfb9a"
            username = "Username"
        },
    )

    // Abort transaction
    throw Exception("An exception is returned, transaction should be aborted")
}

Even though an exception was thrown, the object would still be added to the database.

With this change, if an exception is thrown during a transaction, all writes within that transaction are rolled back.

Here's an example of how this works:

assertFailsWith<SomeException> {
    database.writeTransaction("users") {
        objectStore("users").add(
            jso {
                id = "7740f7c4-f889-498a-bc6d-f88dabdcfb9a"
                username = "Username"
            },
        )

        // Abort transaction
        throw SomeException()
    }
}

// Because the transaction is aborted, no new values should be stored
val users = database.transaction("users") {
    objectStore("users").getAll().toList()
}

assertEquals(listOf(), users)

[jacek-marchwicki]

note: commit was added later to browser, so it might not be always available

https://developer.mozilla.org/en-US/docs/Web/API/IDBTransaction#browser_compatibility

note: Also from the commit documentation we can read:

For an active transaction, commits the transaction. Note that this doesn't normally have to be called — a transaction will automatically commit when all outstanding requests have been satisfied and no new requests have been made. commit() can be used to start the commit process without waiting for events from outstanding requests to be dispatched.

So calling it is optional.

[jacek-marchwicki]

info: I've added this check, so continue() is not called after flow was closed.

Otherwise in the following code:

val returnedValue = database.transaction("customers") {
            objectStore("customers")
                .index("name")
                .openCursor(autoContinue = true)
                .first()
        }

Cursor might try to return new value after first() and after transaction is finished. This will cause TransactionInactiveError.

[jacek-marchwicki]

@Phoenix7351, @cedrickcooke , @twyatt, sorry for the initial fail with the PR. Now it's fixed.

I don't know how to fix the following check:

I guess I should add a label to PR, but this is not allowed by the external contributors.

[twyatt]

I guess I should add a label to PR, but this is not allowed by the external contributors.

Ya, we'll add a semver label when we review it (it helps us later decide what version to release under).

[twyatt]

Also, thanks for being so thorough with this PR. Great work!

[cedrickcooke]

Awesome work on this, thank you

[twyatt]

I'm not sure what the standard approach is for this (as JavaScript is outside my wheelhouse) but I've often seen these kind of checks written as:

Suggested change

	if (transaction.unsafeCast<Json>()["commit"] != undefined) {
	if (typeof transaction.commit === 'function') {

..if the above approach is functionally the same, I think I'd prefer it from a readability perspective. 🤷

Let me know your thoughts / rationale behind the casting to Json.

[jacek-marchwicki]

Thanks! The prosed solution looks much better. I'll check if it works as expected, then prepare update.

[jacek-marchwicki]

@twyatt I've tried with jsTypeOf(transaction::commit) === "function" but this returns true, no matter if the commit function is implement in the web browser or if it's not. Even tho this looks the best, it doesn't work :/

I can use the following code:

val transaction = transaction
if (js("typeof transaction.commit === 'function'"))

but this code is as ugly as the code with unsafeCast ;)

I don't have much preference, so I'm open for your opinion. Maybe you see another solution for this problem?

[jacek-marchwicki]

There is also:

jsTypeOf(transaction.unsafeCast<Json>()["commit"]) === "function"

but this is similar to

transaction.unsafeCast<Json>()["commit"] != undefined

[jacek-marchwicki]

Yet another solution that I found, but still a little bit hacky:

private inline fun <T> functionExists(obj: T, function: T.() -> KFunction<*>): Boolean = 
    jsTypeOf(obj.unsafeCast<Json>()[obj.function().name]) === "function"

Then:

if (functionExists(transaction) { ::commit }) {
    transaction.commit()
}

[twyatt]

I've tried with jsTypeOf(transaction::commit) === "function" but this returns true, no matter if the commit function is implement in the web browser or if it's not. Even tho this looks the best, it doesn't work :/

Oh bummer. Thanks for giving it a try. Let me experiment a little over the weekend and get back to you.

[twyatt]

@jacek-marchwicki how are you testing when commit isn't supported? It looks like all modern browsers support it. Are you downloading an older version of one of the browsers?

[twyatt]

I'm not sure how to test this (with commit specifically), but if you have a moment, maybe one last thing to try:

Note

This is the way that the coroutines library is checking for feature support.

if (jsTypeOf(transaction.asDynamic().commit) != "undefined") {
    transaction.commit()
}

or...

if (jsTypeOf(transaction.asDynamic().commit) === "function") {
    transaction.commit()
}

[jacek-marchwicki]

I'm testing just by not using "commit" name, but temporarily renaming the method to something else that doesn't exist. I.e. "commit2".

[jacek-marchwicki]

Btw, I decided to do the check for "commit" method, because even tho it is supported in the newest web browsers, it was introduced later than other features:

[twyatt]

@jacek-marchwicki thanks for the contribution!