[FIX] [IMP] mail: mail_message: when checking that access rights are not
authorThibault Delavallée <tde@openerp.com>
Fri, 22 Aug 2014 11:36:31 +0000 (13:36 +0200)
committerThibault Delavallée <tde@openerp.com>
Fri, 22 Aug 2014 11:36:54 +0000 (13:36 +0200)
violated in _search, do it in SQL to speedup the query. Indeed doing it via search and
browsing the results to validate the various rules is quite costly.

addons/mail/mail_message.py

index 2529c4b..439b2be 100644 (file)
@@ -603,7 +603,7 @@ class mail_message(osv.Model):
         return allowed_ids
 
     def _search(self, cr, uid, args, offset=0, limit=None, order=None,
-        context=None, count=False, access_rights_uid=None):
+                context=None, count=False, access_rights_uid=None):
         """ Override that adds specific access rights of mail.message, to remove
             ids uid could not see according to our custom rules. Please refer
             to check_access_rule for more details about those rules.
@@ -616,10 +616,12 @@ class mail_message(osv.Model):
         """
         # Rules do not apply to administrator
         if uid == SUPERUSER_ID:
-            return super(mail_message, self)._search(cr, uid, args, offset=offset, limit=limit, order=order,
+            return super(mail_message, self)._search(
+                cr, uid, args, offset=offset, limit=limit, order=order,
                 context=context, count=count, access_rights_uid=access_rights_uid)
         # Perform a super with count as False, to have the ids, not a counter
-        ids = super(mail_message, self)._search(cr, uid, args, offset=offset, limit=limit, order=order,
+        ids = super(mail_message, self)._search(
+            cr, uid, args, offset=offset, limit=limit, order=order,
             context=context, count=False, access_rights_uid=access_rights_uid)
         if not ids and count:
             return 0
@@ -630,14 +632,20 @@ class mail_message(osv.Model):
         author_ids, partner_ids, allowed_ids = set([]), set([]), set([])
         model_ids = {}
 
-        messages = super(mail_message, self).read(cr, uid, ids, ['author_id', 'model', 'res_id', 'notified_partner_ids'], context=context)
-        for message in messages:
-            if message.get('author_id') and message.get('author_id')[0] == pid:
-                author_ids.add(message.get('id'))
-            elif pid in message.get('notified_partner_ids'):
-                partner_ids.add(message.get('id'))
-            elif message.get('model') and message.get('res_id'):
-                model_ids.setdefault(message.get('model'), {}).setdefault(message.get('res_id'), set()).add(message.get('id'))
+        # check read access rights before checking the actual rules on the given ids
+        super(mail_message, self).check_access_rights(cr, access_rights_uid or uid, 'read')
+
+        cr.execute("""SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, n.partner_id
+            FROM "%s" m LEFT JOIN "mail_notification" n
+            ON n.message_id=m.id AND n.partner_id = (%%s)
+            WHERE m.id = ANY (%%s)""" % self._table, (pid, ids,))
+        for id, rmod, rid, author_id, partner_id in cr.fetchall():
+            if author_id == pid:
+                author_ids.add(id)
+            elif partner_id == pid:
+                partner_ids.add(id)
+            elif rmod and rid:
+                model_ids.setdefault(rmod, {}).setdefault(rid, set()).add(id)
 
         allowed_ids = self._find_allowed_doc_ids(cr, uid, model_ids, context=context)
         final_ids = author_ids | partner_ids | allowed_ids
@@ -699,20 +707,20 @@ class mail_message(osv.Model):
         author_ids = []
         if operation == 'read' or operation == 'write':
             author_ids = [mid for mid, message in message_values.iteritems()
-                if message.get('author_id') and message.get('author_id') == partner_id]
+                          if message.get('author_id') and message.get('author_id') == partner_id]
         elif operation == 'create':
             author_ids = [mid for mid, message in message_values.iteritems()
-                if not message.get('model') and not message.get('res_id')]
+                          if not message.get('model') and not message.get('res_id')]
 
         # Parent condition, for create (check for received notifications for the created message parent)
         notified_ids = []
         if operation == 'create':
             parent_ids = [message.get('parent_id') for mid, message in message_values.iteritems()
-                if message.get('parent_id')]
+                          if message.get('parent_id')]
             not_ids = not_obj.search(cr, SUPERUSER_ID, [('message_id.id', 'in', parent_ids), ('partner_id', '=', partner_id)], context=context)
             not_parent_ids = [notif.message_id.id for notif in not_obj.browse(cr, SUPERUSER_ID, not_ids, context=context)]
             notified_ids += [mid for mid, message in message_values.iteritems()
-                if message.get('parent_id') in not_parent_ids]
+                             if message.get('parent_id') in not_parent_ids]
 
         # Notification condition, for read (check for received notifications and create (in message_follower_ids)) -> could become an ir.rule, but not till we do not have a many2one variable field
         other_ids = set(ids).difference(set(author_ids), set(notified_ids))
@@ -729,10 +737,10 @@ class mail_message(osv.Model):
                     ('res_model', '=', doc_model),
                     ('res_id', 'in', list(doc_ids)),
                     ('partner_id', '=', partner_id),
-                    ], context=context)
+                ], context=context)
                 fol_mids = [follower.res_id for follower in fol_obj.browse(cr, SUPERUSER_ID, fol_ids, context=context)]
                 notified_ids += [mid for mid, message in message_values.iteritems()
-                    if message.get('model') == doc_model and message.get('res_id') in fol_mids]
+                                 if message.get('model') == doc_model and message.get('res_id') in fol_mids]
 
         # CRUD: Access rights related to the document
         other_ids = other_ids.difference(set(notified_ids))
@@ -746,15 +754,15 @@ class mail_message(osv.Model):
             else:
                 self.pool['mail.thread'].check_mail_message_access(cr, uid, mids, operation, model_obj=model_obj, context=context)
             document_related_ids += [mid for mid, message in message_values.iteritems()
-                if message.get('model') == model and message.get('res_id') in mids]
+                                     if message.get('model') == model and message.get('res_id') in mids]
 
         # Calculate remaining ids: if not void, raise an error
         other_ids = other_ids.difference(set(document_related_ids))
         if not other_ids:
             return
         raise orm.except_orm(_('Access Denied'),
-                            _('The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: %s, Operation: %s)') % \
-                            (self._description, operation))
+                             _('The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: %s, Operation: %s)') %
+                             (self._description, operation))
 
     def _get_record_name(self, cr, uid, values, context=None):
         """ Return the related document name, using name_get. It is done using