From d29b1554b04643dceb82361d709f1f890a110ce6 Mon Sep 17 00:00:00 2001 From: Patrick Samson Date: Wed, 17 Jul 2013 17:31:02 +0200 Subject: [PATCH] Redesign the DB queries to fix issue #15 ; Pre-release of v3.0.0 --- CHANGELOG | 15 +++++-- docs/conf.py | 4 +- postman/__init__.py | 2 +- postman/models.py | 33 ++++++---------- postman/query.py | 96 +++++++++++++++++++++++++++++++++++++++++++++ postman/tests.py | 16 ++++---- 6 files changed, 131 insertions(+), 35 deletions(-) create mode 100644 postman/query.py diff --git a/CHANGELOG b/CHANGELOG index 53a2d70..ef98f13 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,14 +2,23 @@ Django Postman changelog ======================== +Version 3.0.0, July 2013 +------------------------ +* !MAJOR! Redesign the DB queries for the 'by conversation' mode, + to fix the performances problem of issue #15. + Note that the counting of messages by thread is no more global (all folders) + but is now limited to the only targeted folder. +* Extend the support of django-notification from version 0.2.0 to 1.0. +* Avoid the 'Enter text to search.' help text imposed in version 1.2.5 of ajax_select. + Version 2.1.1, December 2012 ---------------------------- -* Fix issue #21, a missing unicode/str encoding migration +* Fix issue #21, a missing unicode/str encoding migration. Version 2.1.0, December 2012 ---------------------------- -* Make the app compatible with the new 'Custom Auth Model' feature of Django 1.5 -* Add a setting: POSTMAN_SHOW_USER_AS +* Make the app compatible with the new 'Custom Auth Model' feature of Django 1.5. +* Add a setting: POSTMAN_SHOW_USER_AS. * Remove the dependency to django-pagination in the default template set. * Add an optional auto_moderators parameter to the pm_write() API function. * Add a template for the autocomplete of multiple recipients in version 1.2.x of django-ajax-selects. diff --git a/docs/conf.py b/docs/conf.py index 3d57c3e..63e7ec4 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -45,9 +45,9 @@ copyright = u'2010, Patrick Samson' # built documents. # # The short X.Y version. -version = '2.2' +version = '3.0' # The full version, including alpha/beta/rc tags. -release = '2.2.0a1' +release = '3.0.0a1' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/postman/__init__.py b/postman/__init__.py index 7961887..eef3e20 100644 --- a/postman/__init__.py +++ b/postman/__init__.py @@ -4,7 +4,7 @@ A messaging application for Django from __future__ import unicode_literals # following PEP 386: N.N[.N]+[{a|b|c|rc}N[.N]+][.postN][.devN] -VERSION = (2, 2, 0) +VERSION = (3, 0, 0) PREREL = ('a', 1) POST = 0 DEV = 0 diff --git a/postman/models.py b/postman/models.py index 9da2e28..fbe514b 100644 --- a/postman/models.py +++ b/postman/models.py @@ -8,6 +8,7 @@ except ImportError: from postman.future_1_5 import get_user_model from django.core.exceptions import ValidationError from django.db import models +from django.db.models.query import QuerySet try: from django.utils.text import Truncator # Django 1.4 except ImportError: @@ -19,6 +20,7 @@ except ImportError: from datetime import datetime now = datetime.now +from postman.query import PostmanQuery from postman.urls import OPTION_MESSAGES from postman.utils import email_visitor, notify_user @@ -41,9 +43,6 @@ ORDER_BY_FIELDS = { } ORDER_BY_MAPPER = {'sender': 'f', 'recipient': 't', 'subject': 's', 'date': 'd'} # for templatetags usage -dbms = settings.DATABASES['default']['ENGINE'].rsplit('.', 1)[-1] -QUOTE_CHAR = '`' if dbms == 'mysql' else '"' - def get_order_by(query_dict): """ @@ -84,18 +83,11 @@ def get_user_representation(user): class MessageManager(models.Manager): """The manager for Message.""" - @property - def _last_in_thread(self): - """Return the latest message id for each conversation.""" - return self.filter(thread__isnull=False).values('thread').annotate(models.Max('pk'))\ - .values_list('pk__max', flat=True).order_by() - def _folder(self, related, filters, option=None, order_by=None): """Base code, in common to the folders.""" + qs = self.all() if option == OPTION_MESSAGES else QuerySet(self.model, PostmanQuery(self.model), using=self._db) if related: - qs = self.select_related(*related) - else: - qs = self.all() + qs = qs.select_related(*related) if order_by: qs = qs.order_by(order_by) if isinstance(filters, (list, tuple)): @@ -110,15 +102,14 @@ class MessageManager(models.Manager): # should not be necessary. Otherwise add: # .extra(select={'count': 'SELECT 1'}) else: - return qs.filter( - models.Q(id__in=self._last_in_thread.filter(lookups)) | models.Q(lookups, thread__isnull=True) - ).extra(select={'count': QUOTE_CHAR.join([ - 'SELECT COUNT(*) FROM ', 'postman_message', ' T' - ' WHERE T.', 'thread_id', ' = ', 'postman_message', '.', 'thread_id', ' ' - ])}) - # For single message, 'count' is returned as 0. Should be acceptable if known. - # If not, replace "COUNT(*)" by "1+COUNT(*)" and add: - # ' AND T."id" <> T."thread_id"' + qs = qs.extra(select={'count': '{0}.count'.format(qs.query.pm_alias_prefix)}) + qs.query.pm_set_extra(table=( + self.filter(lookups, thread_id__isnull=True).extra(select={'count': 0})\ + .values_list('id', 'count').order_by(), + self.filter(lookups, thread_id__isnull=False).values('thread').annotate(id=models.Max('pk'), count=models.Count('pk'))\ + .values_list('id', 'count').order_by(), + )) + return qs def inbox(self, user, related=True, **kwargs): """ diff --git a/postman/query.py b/postman/query.py new file mode 100644 index 0000000..59dd04e --- /dev/null +++ b/postman/query.py @@ -0,0 +1,96 @@ +from __future__ import unicode_literals +import new +from types import MethodType + +from django.db.models.sql.query import Query + + +class Proxy(object): + """ + Code base for an instance proxy. + """ + + def __init__(self, target): + self._target = target + + def __getattr__(self, name): + target = self._target + f = getattr(target, name) + if isinstance(f, MethodType): + return new.instancemethod(f.im_func, self, target.__class__) + else: + return f + + def __setattr__(self, name, value): + if name != '_target': + setattr(self._target, name, value) + else: + object.__setattr__(self, name, value) + + +class CompilerProxy(Proxy): + """ + A proxy to a compiler. + """ + + # @Override + def as_sql(self, *args, **kwargs): + sql, params = self._target.as_sql(*args, **kwargs) + # mimics compiler.py/SQLCompiler/get_from_clause() and as_sql() + qn = self.quote_name_unless_alias + qn2 = self.connection.ops.quote_name + alias = self.query.tables[0] + name, alias, join_type, lhs, lhs_col, col, nullable = self.query.alias_map[alias] + alias_str = (alias != name and ' {0}'.format(alias) or '') + clause = 'FROM {0}{1}'.format(qn(name), alias_str) + index = sql.index(clause) + len(clause) + extra_table, extra_params = self.union(self.query.pm_get_extra()) + new_sql = [ + sql[:index], + ' {0} ({1}) {2} ON ({3}.{4} = {2}.{5})'.format( + self.query.INNER, extra_table, self.query.pm_alias_prefix, qn(alias), qn2('id'), qn2('id')), + ] + if index < len(sql): + new_sql.append(sql[index:]) + new_sql = ''.join(new_sql) + return new_sql, extra_params + params + + def union(self, querysets): + """ + Join several querysets by a UNION clause. Returns the SQL string and the list of parameters. + """ + result_sql, result_params = [], [] + for qs in querysets: + sql, params = qs.query.sql_with_params() + result_sql.append(sql) + result_params.extend(params) + return ' UNION '.join(result_sql), tuple(result_params) + + +class PostmanQuery(Query): + """ + A custom SQL query. + """ + pm_alias_prefix = 'PM' + + # @Override + def __init__(self, *args, **kwargs): + super(PostmanQuery, self).__init__(*args, **kwargs) + self._pm_table = None + + # @Override + def clone(self, *args, **kwargs): + obj = super(PostmanQuery, self).clone(*args, **kwargs) + obj._pm_table = self._pm_table + return obj + + # @Override + def get_compiler(self, *args, **kwargs): + compiler = super(PostmanQuery, self).get_compiler(*args, **kwargs) + return CompilerProxy(compiler) + + def pm_set_extra(self, table): + self._pm_table = table + + def pm_get_extra(self): + return self._pm_table diff --git a/postman/tests.py b/postman/tests.py index 1251b24..3aec90a 100644 --- a/postman/tests.py +++ b/postman/tests.py @@ -76,7 +76,7 @@ class GenericTest(TestCase): Usual generic tests. """ def test_version(self): - self.assertEqual(sys.modules['postman'].__version__, "2.2.0a1") + self.assertEqual(sys.modules['postman'].__version__, "3.0.0a1") class BaseTest(TestCase): @@ -985,10 +985,10 @@ class MessageManagerTest(BaseTest): self.assertQuerysetEqual(Message.objects.trash(self.user1, option=OPTION_MESSAGES), [], transform=pk) self.assertQuerysetEqual(Message.objects.trash(self.user2, option=OPTION_MESSAGES), [], transform=pk) # by conversations - self.assertQuerysetEqual(Message.objects.sent(self.user1), [(m7.pk,0),(m6.pk,0),(m5.pk,3),(m2.pk,0),(m1.pk,0)], transform=pk_cnt) - self.assertQuerysetEqual(Message.objects.sent(self.user2), [(m10.pk,0),(m9.pk,0),(m8.pk,0),(m4.pk,3)], transform=pk_cnt) - self.assertQuerysetEqual(Message.objects.inbox(self.user1), [(m8.pk,0),(m4.pk,3)], transform=pk_cnt) - self.assertQuerysetEqual(Message.objects.inbox(self.user2), [(m7.pk,0),(m6.pk,0),(m5.pk,3)], transform=pk_cnt) + self.assertQuerysetEqual(Message.objects.sent(self.user1), [(m7.pk,0),(m6.pk,0),(m5.pk,2),(m2.pk,0),(m1.pk,0)], transform=pk_cnt) + self.assertQuerysetEqual(Message.objects.sent(self.user2), [(m10.pk,0),(m9.pk,0),(m8.pk,0),(m4.pk,1)], transform=pk_cnt) + self.assertQuerysetEqual(Message.objects.inbox(self.user1), [(m8.pk,0),(m4.pk,1)], transform=pk_cnt) + self.assertQuerysetEqual(Message.objects.inbox(self.user2), [(m7.pk,0),(m6.pk,0),(m5.pk,2)], transform=pk_cnt) self.assertQuerysetEqual(Message.objects.thread(self.user1, Q(thread=m3.pk)), [m3.pk,m4.pk,m5.pk], transform=pk) self.assertQuerysetEqual(Message.objects.thread(self.user1, Q(pk=m4.pk)), [m4.pk], transform=pk) @@ -1031,10 +1031,10 @@ class MessageManagerTest(BaseTest): self.assertQuerysetEqual(Message.objects.inbox(self.user1, option=OPTION_MESSAGES), [m4.pk], transform=pk) self.assertQuerysetEqual(Message.objects.inbox(self.user2, option=OPTION_MESSAGES), [m5.pk,m3.pk], transform=pk) # by conversations - self.assertQuerysetEqual(Message.objects.sent(self.user1), [(m7.pk,0),(m5.pk,3)], transform=pk_cnt) + self.assertQuerysetEqual(Message.objects.sent(self.user1), [(m7.pk,0),(m5.pk,1)], transform=pk_cnt) self.assertQuerysetEqual(Message.objects.sent(self.user2), [(m8.pk,0)], transform=pk_cnt) - self.assertQuerysetEqual(Message.objects.inbox(self.user1), [(m4.pk,3)], transform=pk_cnt) - self.assertQuerysetEqual(Message.objects.inbox(self.user2), [(m5.pk,3)], transform=pk_cnt) + self.assertQuerysetEqual(Message.objects.inbox(self.user1), [(m4.pk,1)], transform=pk_cnt) + self.assertQuerysetEqual(Message.objects.inbox(self.user2), [(m5.pk,2)], transform=pk_cnt) self.assertQuerysetEqual(Message.objects.thread(self.user1, Q(thread=m3.pk)), [m3.pk,m4.pk,m5.pk], transform=pk) self.assertQuerysetEqual(Message.objects.thread(self.user1, Q(pk=m4.pk)), [m4.pk], transform=pk) -- 2.39.5