From 863b361707c4f2990e05bf798d2f76b199326318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 22 Aug 2014 13:36:31 +0200 Subject: [PATCH] [FIX] [IMP] mail: mail_message: when checking that access rights are not 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 | 48 +++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py index 2529c4b8820..439b2bed876 100644 --- a/addons/mail/mail_message.py +++ b/addons/mail/mail_message.py @@ -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