Merge pull request #43 from VVyacheslav/django-testcase-support-and-f…

…ew-improvements

Added django TestCase support and few improvements.

.gitignore

@@ -1,3 +1,4 @@
 *.pyc
 .idea
 *.egg-info
+/tmp/

Dockerfile

@@ -0,0 +1,8 @@
+FROM python:3.8
+
+WORKDIR /app
+
+RUN pip install --upgrade pip
+
+COPY ./test-requires.txt /tmp/test-requires.txt
+RUN pip install -r /tmp/test-requires.txt

README.md

@@ -131,6 +131,12 @@
     TestModel.objects.filter(attr_bool=True).update(attr_uint=2)
     ```
 
+## Run tests
+
+```shell
+docker-compose up django
+```
+
 ## Notes for production usage
 
 * Sphinxsearch engine has some issues with SQL-syntax support, and they vary

docker-compose.yml

@@ -7,4 +7,17 @@ services:
             - "9307:9307"
         volumes:
             - "./test_config/:/opt/sphinx/conf/"
-            - "/tmp/:/sphinxdata/indexes/"
+            - "./tmp/:/sphinxdata/indexes/"
+
+    django:
+        build:
+            context: .
+            dockerfile: Dockerfile
+        container_name: django_sphinx
+        command: python manage.py test -v 1 --noinput
+        depends_on:
+            - sphinxsearch
+        volumes:
+            - "./:/app"
+        ports:
+            - "8005:8005"

sphinxsearch/backend/sphinx/base.py

@@ -1,4 +1,5 @@
 import sys
+import warnings
 from collections import OrderedDict
 
 from django.db import ProgrammingError
@@ -132,7 +133,7 @@ class SphinxFeatures(base.DatabaseFeatures):
     # support it.
     supports_transactions = True
     allows_group_by_pk = False
-    uses_savepoints = False
+    uses_savepoints = True
     supports_column_check_constraints = False
     is_sql_auto_is_null_enabled = False
 
@@ -155,3 +156,25 @@ def mysql_version(self):
             raise Exception('Unable to determine MySQL version from version '
                             'string %r' % server_info)
         return tuple(int(x) for x in match.groups())
+
+    def _savepoint(self, sid):
+        """ Stub for savepoints.
+
+        Sphinx does not support transactions Django knows, so we have to clean
+        Sphinx database by ourselves between tests. But we don't want tell
+        Django about it, because it will not be using them for another
+        databases as we need. So we tell Django we support transaction and
+        savepoints.
+        When transactions are active Django does rollback to a savepoint after
+        each test. So we need this and two stubs below that Django
+        thinks everything is fine."""
+        warnings.warn("Sphinx warning: Sphinx doesn't support savepoints, "
+                      "only emulates them. Savepoint creation is not done.")
+
+    def _savepoint_rollback(self, sid):
+        warnings.warn("Sphinx warning: Sphinx doesn't support savepoints, "
+                      "only emulates them. Rollback is not done.")
+
+    def _savepoint_commit(self, sid):
+        warnings.warn("Sphinx warning: Sphinx doesn't support savepoints, "
+                      "only emulates them. Commit is not done.")

sphinxsearch/backend/sphinx/compiler.py

@@ -88,18 +88,22 @@ def ensure_list(s):
     def as_sql(self, with_limits=True, with_col_aliases=False, subquery=False):
         """ Patching final SQL query."""
         query = self.query
-        # replacing WHERE node with sphinx-aware node implementation
-        where, query.where = query.where, sqls.SphinxWhereNode()
-        # Adding MATCH() node to WHERE node if needed
-        self._add_match_extra(query)
-        # moving where conditions to SELECT clause because of better support
-        # of SQL expressions in sphinxsearch.
         try:
-            self._add_where_result(query, where)
+            where_sql, where_params = query.where.as_sql(self, self.connection)
         except EmptyResultSet:
             # Where node compiled to always-false condition, but we still need
             # to call pre_sql_setup() and other methods by super().as_sql
             pass
+        else:
+            # creating WHERE node with sphinx-aware node implementation if
+            # everything is fine
+            where = sqls.SphinxWhereNode()
+            # Adding MATCH() node to WHERE node if needed
+            self._add_match_extra(query, where)
+            query.where = where
+            # Adding where conditions to SELECT clause because of better
+            # support of SQL expressions in sphinxsearch.
+            self._add_where_result(query, where_sql, where_params)
 
         sql, args = super().as_sql(with_limits, with_col_aliases)
 
@@ -140,11 +144,10 @@ def as_sql(self, with_limits=True, with_col_aliases=False, subquery=False):
             args += tuple(values)
         return sql, args
 
-    def _add_where_result(self, query, where):
-        where_sql, where_params = where.as_sql(self, self.connection)
+    def _add_where_result(self, query, where_sql, where_params):
+        # Without annotation queryset.count() receives 1 as where_result
+        # and count it as aggregation result.
         if where_sql:
-            # Without annotation queryset.count() receives 1 as where_result
-            # and count it as aggregation result.
             query.add_annotation(
                 sqls.SphinxWhereExpression(where_sql, where_params),
                 '__where_result')
@@ -170,7 +173,7 @@ def get_group_ordering(self):
             result.append("%s %s" % (col, order))
         return " WITHIN GROUP ORDER BY " + ", ".join(result)
 
-    def _add_match_extra(self, query):
+    def _add_match_extra(self, query, where):
         """ adds MATCH clause to query.where """
         # Adding match node if needed
         match = getattr(query, 'match', None)
@@ -208,7 +211,7 @@ def decode(s):
         match_expr = u"MATCH('%s')" % u' '.join(map(decode, expression))
 
         # add MATCH() to query.where
-        query.where.add(sqls.SphinxExtraWhere([match_expr], []), AND)
+        where.add(sqls.SphinxExtraWhere([match_expr], []), AND)
 
 
 # Set SQLCompiler appropriately, so queries will use the correct compiler.
@@ -248,12 +251,7 @@ def as_sql(self):
         if node and need_replace:
             sql, args = self.as_replace(node)
         else:
-
-            match = getattr(self.query, 'match', None)
-            if match:
-                # add match extra where
-                self._add_match_extra(match)
-
+            self._add_match_extra(self.query, self.query.where)
             sql, args = super().as_sql()
         return sql, args
 

sphinxsearch/models.py

@@ -1,4 +1,5 @@
 # coding: utf-8
+import warnings
 from copy import copy
 
 from django.conf import settings
@@ -228,6 +229,15 @@ def _fetch_all(self):
             if getattr(self.query, 'with_meta', False):
                 self._fetch_meta()
 
+    def select_for_update(self, nowait=False, skip_locked=False, of=()):
+        """ Sphinx doesn't support select_for_update, so make stub.
+
+        That method is not usefull for search index but is used by many others
+        queryset methods, for example update_or_create.
+        """
+        warnings.warn("Sphinx warning: select_for_update doesn't do a lock.")
+        return self
+
 
 class SphinxManager(models.Manager):
     use_for_related_fields = True

testproject/settings.py

@@ -79,13 +79,13 @@
 DATABASES = {
     SPHINX_DATABASE_NAME: {
         'ENGINE': 'sphinxsearch.backend.sphinx',
-        'HOST': '127.0.0.1',
+        'HOST': 'sphinxsearch',
         'PORT': 9307,
         'NAME': 'sphinx',
     },
     "cloned": {
         'ENGINE': 'sphinxsearch.backend.sphinx',
-        'HOST': '127.0.0.1',
+        'HOST': 'sphinxsearch',
         'PORT': 9307,
         'NAME': 'sphinx_x',
     }

testproject/testapp/tests.py

@@ -4,26 +4,22 @@
 import pytz
 import re
 from django.conf import settings
-from django.db import connections
+from django.db import connections, transaction
 from django.db.models import Sum, Q
 from django.db.utils import ProgrammingError
-from django.test import TransactionTestCase, utils
+from django.test import utils, TestCase
 from django.utils import timezone
 
 from sphinxsearch.routers import SphinxRouter
 from sphinxsearch.utils import sphinx_escape
 from testapp import models
 
 
-class SphinxModelTestCaseBase(TransactionTestCase):
+class SphinxModelTestCaseBase(TestCase):
     _id = 0
 
     model = models.TestModel
 
-    def _fixture_teardown(self):
-        # Prevent SHOW FULL TABLES call
-        pass
-
     def truncate_model(self):
         conn = connections[settings.SPHINX_DATABASE_NAME]
         try:
@@ -122,6 +118,11 @@ def testLenOfEmptySet(self):
         self.assertEqual(qs.count(), 0)
         self.assertEqual(len(qs[:0]), 0)
 
+    def testLenOfAnotherEmptySet(self):
+        qs = self.model.objects.all()[0:0]
+        self.assertEqual(len(qs[0:0]), 0)
+        self.assertEqual(qs.count(), 0)
+
     def testGroupByExtraSelect(self):
         qs = self.model.objects.all()
 
@@ -237,6 +238,18 @@ def testBulkUpdate(self):
         other = self.reload_object(self.obj)
         self.assertFalse(other.attr_bool)
 
+    def testUpdateOrCreate(self):
+        exp= (({'id': self.defaults['id']}, {'attr_uint': 555}),
+              ({'id': self.new_id()}, {'attr_uint': 666}))
+
+        for param in exp:
+            self.model.objects.update_or_create(**param[0], defaults=param[1])
+
+        act = [({'id': o.id}, {'attr_uint': o.attr_uint})
+               for o in self.model.objects.all()]
+
+        self.assertCountEqual(exp, act)
+
     def testDelete(self):
         if self.no_string_compare:  # pragma: no cover
             self.skipTest("searchd version is too low")
@@ -697,3 +710,17 @@ def test_clone_db(self):
 
         obj1 = self.model.objects.using('cloned').get(pk=obj.pk)
         self.assertIsNotNone(obj1)
+
+    def test_transaction_with_savepoint_smoke(self):
+        """
+        Opening and closing transaction with a savepoint doesn't cause error
+        """
+        with transaction.atomic(savepoint=True):
+            pass
+
+    def test_transaction_without_savepoint_smoke(self):
+        """
+        Opening and closing transaction without a savepoint doesn't cause error
+        """
+        with transaction.atomic(savepoint=False):
+            pass