sdispater / orator

The Orator ORM provides a simple yet beautiful ActiveRecord implementation.
https://orator-orm.com
MIT License
1.43k stars 173 forks source link

Nested transactions don't appear to function properly #180

Open denislins opened 7 years ago

denislins commented 7 years ago

I have a modified version of unittest's TestCase, in which I start a transaction before each test, and rollback the transaction after each test. This makes sure every test is isolated from each other.

It looks like this:

class AppTestCase(TestCase):
    def setUp(self):
        db.begin_transaction()

    def tearDown(self):
        db.rollback()

The problem is, I'm testing a class which opens a transaction of its own, which causes this error to be raised: psycopg2.ProgrammingError: autocommit cannot be used inside a transaction.

None of my code is executed. Once it attempts to open the second transaction, the error is raised.

It appears to me that the library sets autocommit to False every time a transaction is opened, which causes this issue, but I could be wrong.

Any ideas about how we could solve this?

I'm using the latest version of orator, and postgres as database backend.

sdispater commented 7 years ago

What does your class look like?

I will try to reproduce but having an example might help.

denislins commented 7 years ago

It's very simple, actually:

class CreateCustomer:
    def __init__(self, data):
        self.data = data

    def execute(self):
        with db.transaction():
            customer = self._create_customer()
            self._create_emails(customer)

    def _create_customer(self):
        return Customer.create(self.data['customer'])

    ...
gilvan-reis commented 6 years ago

@sdispater any news about this issue?

pvfrota commented 5 years ago

This error seems to occur here, because for each transaction that is created, the "autocommit" parameter is modified, causing a modification in the current session, which is not allowed by psycopg2.

https://github.com/sdispater/orator/blob/e5d48d64853d6994b3bf451ab8148fac1f3fcb2a/orator/connections/postgres_connection.py#L39-L42

A possible solution could be this:

def begin_transaction(self):
    if not self._transactions:
        self._connection.autocommit = False

    super(PostgresConnection, self).begin_transaction()

However, I couldn't quite understand how Orator handles this "nested transactions" issue. It seems to me that it actually keeps them all in one transaction, and the last commit or rollback command that will be the only one executed. Correct me if I'm wrong, but if that's right, it seems to me to be somewhat dangerous, and can cause some unexpected behavior.

I say this because of the code snippets where transactions are created, committed and rolled back:

https://github.com/sdispater/orator/blob/e5d48d64853d6994b3bf451ab8148fac1f3fcb2a/orator/connections/postgres_connection.py#L39-L42

https://github.com/sdispater/orator/blob/e5d48d64853d6994b3bf451ab8148fac1f3fcb2a/orator/connections/connection.py#L316-L317

https://github.com/sdispater/orator/blob/e5d48d64853d6994b3bf451ab8148fac1f3fcb2a/orator/connections/postgres_connection.py#L44-L49

https://github.com/sdispater/orator/blob/e5d48d64853d6994b3bf451ab8148fac1f3fcb2a/orator/connections/postgres_connection.py#L51-L58