linuxlewis / djorm-ext-pgfulltext

PostgreSQL full-text search integration with django orm.
Other
250 stars 84 forks source link

Djano 1.6 transaction management changes and TransactionManagement error when working in admin #10

Closed neara closed 10 years ago

neara commented 10 years ago

Since django 1.6, the orm submits all sql queries when they are called.

I didn't have any problems when working with the model, that has search field, outside of admin, but in admin, django enforces atomic validation and saving the model would fail with Transaction management error

Traceback (most recent call last):
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\contrib\staticfiles\handlers.py", line 67, in __call__
    return self.application(environ, start_response)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\core\handlers\wsgi.py", line 206, in __call__
    response = self.get_response(request)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\core\handlers\base.py", line 196, in get_response
    response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\core\handlers\base.py", line 231, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django_extensions\management\technical_response.py", line 5, in null_technical_500_response
    six.reraise(exc_type, exc_value, tb)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\core\handlers\base.py", line 114, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\contrib\admin\options.py", line 430, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\utils\decorators.py", line 99, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\views\decorators\cache.py", line 52, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\contrib\admin\sites.py", line 198, in inner
    return view(request, *args, **kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\utils\decorators.py", line 29, in _wrapper
    return bound_func(*args, **kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\utils\decorators.py", line 99, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\utils\decorators.py", line 25, in bound_func
    return func(self, *args2, **kwargs2)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\db\transaction.py", line 339, in inner
    return func(*args, **kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\contrib\admin\options.py", line 1228, in change_view
    self.save_model(request, new_object, form, True)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\contrib\admin\options.py", line 858, in save_model
    obj.save()
  File "C:\Development\fanwaze\ArenaWaze\src\arena\fansite\models.py", line 72, in save
    super(FanSite, self).save(*args, **kwargs)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\db\models\base.py", line 545, in save
    force_update=force_update, update_fields=update_fields)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\db\models\base.py", line 582, in save_base
    update_fields=update_fields, raw=raw, using=using)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\dispatch\dispatcher.py", line 185, in send
    response = receiver(signal=self, sender=sender, **named)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\djorm_pgfulltext\models.py", line 14, in auto_update_search_field_handler
    instance.update_search_field()
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\djorm_pgfulltext\models.py", line 75, in update_search_field
    self._fts_manager.update_search_field(pk=self.pk, using=using, config=config)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\djorm_pgfulltext\models.py", line 196, in update_search_field
    transaction.enter_transaction_management(using=using)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\db\transaction.py", line 70, in enter_transaction_management
    get_connection(using).enter_transaction_management(managed, forced)
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\db\backends\__init__.py", line 280, in enter_transaction_management
    self.validate_no_atomic_block()
  File "C:\Development\fanwaze\ArenaWaze\venv\lib\site-packages\django\db\backends\__init__.py", line 360, in validate_no_atomic_block
    "This is forbidden when an 'atomic' block is active.")
TransactionManagementError: This is forbidden when an 'atomic' block is active.

This error lead me to think that something was trying to manage the transaction and/or open new transaction, while the current block is active.

I took the liberty of diving into the code, changing update_search_field() in SearchManagerMixin, fixed this issue. These are the changes i introduced:

class SearchManagerMixIn(object):
    def update_search_field(self, pk=None, config=None, using=None):
        '''
        Update the search_field of one instance, or a list of instances, or
        all instances in the table (pk is one key, a list of keys or none).

        If there is no search_field, this function does nothing.
        '''

        if not self.search_field:
            return

        if not config:
            config = self.config

        if using is None:
            using = self.db

        connection = connections[using]
        qn = connection.ops.quote_name

        where_sql = ''
        params = []
        if pk is not None:
            if isinstance(pk, (list, tuple)):
                params = pk
            else:
                params = [pk]

            where_sql = "WHERE %s IN (%s)" % (
                qn(self.model._meta.pk.column),
                ','.join(repeat("%s", len(params)))
            )

        search_vector = self._get_search_vector(config, using)
        sql = "UPDATE %s SET %s = %s %s;" % (
            qn(self.model._meta.db_table),
            qn(self.search_field),
            search_vector,
            where_sql
        )

        # if not transaction.is_managed(using=using):
        #     transaction.enter_transaction_management(using=using)
        #     forced_managed = True
        # else:
        #     forced_managed = False

        cursor = connection.cursor()
        cursor.execute(sql, params)

        # try:
        #     if forced_managed:
        #         transaction.commit(using=using)
        #     else:
        #         transaction.commit_unless_managed(using=using)
        # finally:
        #     if forced_managed:
        #         transaction.leave_transaction_management(using=using)

I am not an expert on transaction management, but doing this made it all work in django admin. I also checked, and indeed the search_field is being updated as it should.

I didn't send you a pull request with this, as i am not 100% sure this is the proper way to solve this.

What do you think?

niwinz commented 10 years ago

Hi! Thanks for report it!

Commenting this code is not a good solution. It should create some abstraction for transaction management with distinct api's. I will fix it as sooner as possible!

fabiociotoli commented 10 years ago

I'm getting the same error with django 1.6. Many thanks for working on it @niwibe !

niwinz commented 10 years ago

I just commited a fix: https://github.com/niwibe/djorm-ext-pgfulltext/commit/d3db806e2cee3fc848ab2b9d13a7f86d75b21690

Can you test if it works properly before release a new version with a fix? A lot of thanks.

neara commented 10 years ago

Works nicely. Admin works ok (saving/creating), all my tests pass as well. So far, don't see any problem ;) Thank you for the fix!

fabiociotoli commented 10 years ago

Work like a charm! Thank you.

niwinz commented 10 years ago

:+1: