Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commit transaction after early return #63

Merged

Conversation

sondrele
Copy link
Contributor

@sondrele sondrele commented Jan 9, 2025

Ever since #57 and transaction was made inline the API has allowed for non-label returns from a transaction block.

This is problematic because it allows for uncommitted transactions.

The provided test case demonstrate the problem:

    @Test
    fun transactionWithNonLabeledReturnGetsCommitted() {
        fun insertAndReturn(session: Session, name: String) {
            session.transaction { tx ->
                tx.run(queryOf(insert, name, Date()).asUpdate)
                // Non-labeled return
                return
            }
        }

        using(sessionOf(testDataSource)) { session ->
            insertAndReturn(session, "Chris")
        }

        using(sessionOf(testDataSource)) { session ->
            val membersCount = session.single(queryOf("select count(*) as count from members")) { row ->
                row.int("count")
            }
            assertEquals(1, membersCount)
        }
    }

Note that changing insertAndReturn to return@transaction works as expected, but since transaction is inline the return statement will exit the insertAndReturn function before the underlying connection gets committed.

A potential solution could have been to mark operation as crossinline (like so inline fun <A> transaction(crossinline operation: (TransactionalSession) -> A): A), but this would break support for coroutines within the transaction block.

I altered the implementation to commit the transaction as part of the finally block instead and it seams to work for this example.

@seratch seratch merged commit 2b8388d into seratch:master Jan 9, 2025
3 checks passed
@seratch seratch added the bug label Jan 9, 2025
@seratch
Copy link
Owner

seratch commented Jan 9, 2025

Thanks for your contribution! I will release a new version tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants