[FIX] expression: proper handling of empty string in m2o domain criterions

Also fixed incorrect name_search() implementations in
res.partner and res.currency, and added corresponding
tests.

lp bug: https://launchpad.net/bugs/861350 fixed

bzr revid: odo@openerp.com-20111011163435-szj9vm6x9x8nryn8

openerp/addons/base/res/res_currency.py

@@ -109,14 +109,14 @@ class res_currency(osv.osv):
     def name_search(self, cr, user, name='', args=None, operator='ilike', context=None, limit=100):
         if not args:
             args = []
-        if name:
-            ids = self.search(cr, user, ([('name','=',name)] + args), limit=limit, context=context)
+        results = super(res_currency,self)\
+            .name_search(cr, user, name, args, operator=operator, context=context, limit=limit)
+        if not results:
             name_match = CURRENCY_DISPLAY_PATTERN.match(name)
-            if not ids and name_match:
-               ids = self.search(cr, user, [('name','=', name_match.group(1))] + args, limit=limit, context=context)
-        else:
-            ids = self.search(cr, user, args, limit=limit, context=context)
-        return self.name_get(cr, user, ids, context=context)
+            if name_match:
+                results = super(res_currency,self)\
+                    .name_search(cr, user, name_match.group(1), args, operator=operator, context=context, limit=limit)
+        return results
 
     def name_get(self, cr, uid, ids, context=None):
         if not ids:

openerp/addons/base/res/res_partner.py

@@ -195,14 +195,13 @@ class res_partner(osv.osv):
 
     def name_search(self, cr, uid, name, args=None, operator='ilike', context=None, limit=100):
         if not args:
-            args=[]
-        if name:
+            args = []
+        # short-circuit ref match when possible
+        if name and operator in ('=', 'ilike', '=ilike', 'like'):
             ids = self.search(cr, uid, [('ref', '=', name)] + args, limit=limit, context=context)
-            if not ids:
-                ids = self.search(cr, uid, [('name', operator, name)] + args, limit=limit, context=context)
-        else:
-            ids = self.search(cr, uid, args, limit=limit, context=context)
-        return self.name_get(cr, uid, ids, context)
+            if ids:
+                return self.name_get(cr, uid, ids, context)
+        return super(res_partner,self).name_search(cr, uid, name, args, operator=operator, context=context, limit=limit)
 
     def _email_send(self, cr, uid, ids, email_from, subject, body, on_error=None):
         partners = self.browse(cr, uid, ids)

openerp/addons/base/test/test_osv_expression.yml

@@ -235,11 +235,14 @@
         res_3.sort()
         res_4 = self.search(cr, uid, [('parent_id', 'not in', [False])]) # get rows with not null parent_id
         res_4.sort()
-        assert res_0 == partner_ids
-        assert res_1 == partner_ids
-        assert res_2 == with_parent
-        assert res_3 == partner_ids
-        assert res_4 == with_parent
+        res_4b = self.search(cr, uid, [('parent_id', 'not ilike', '')]) # get only rows without parent
+        res_4b.sort()
+        assert res_0 == partner_ids, 'res0: expected %r, got %r' % (partner_ids, res_0)
+        assert res_1 == partner_ids, 'res1: expected %r, got %r' % (partner_ids, res_1)
+        assert res_2 == with_parent, 'res2: expected %r, got %r' % (with_parent, res_2)
+        assert res_3 == partner_ids, 'res3: expected %r, got %r' % (partner_ids, res_3)
+        assert res_4 == with_parent, 'res4: expected %r, got %r' % (with_parent, res_4)
+        assert res_4b == without_parent, 'res4b: expected %r, got %r' % (without_parent, res_4b)
         # The results of these queries, when combined with queries 0..4 must
         # give the whole set of ids.
         res_5 = self.search(cr, uid, [('parent_id', 'like', 'probably_unexisting_name')])
@@ -252,11 +255,15 @@
         res_8.sort()
         res_9 = self.search(cr, uid, [('parent_id', 'in', [False])])
         res_9.sort()
-        assert res_5 == []
-        assert res_6 == []
-        assert res_7 == without_parent
-        assert res_8 == []
-        assert res_9 == without_parent
+        res_9b = self.search(cr, uid, [('parent_id', 'ilike', '')]) # get those with a parent
+        res_9b.sort()
+
+        assert res_5 == [], 'res5: expected %r, got %r' % ([], res_5)
+        assert res_6 == [], 'res6: expected %r, got %r' % ([], res_6)
+        assert res_7 == without_parent, 'res7: expected %r, got %r' % (without_parent, res_7)
+        assert res_8 == [], 'res8: expected %r, got %r' % ([], res_8)
+        assert res_9 == without_parent, 'res9: expected %r, got %r' % (without_parent, res_9)
+        assert res_9b == with_parent, 'res9b: expected %r, got %r' % (with_parent, res_9b)
         # These queries must return exactly the results than the queries 0..4,
         # i.e. not ... in ... must be the same as ... not in ... .
         res_10 = self.search(cr, uid, ['!', ('parent_id', 'like', 'probably_unexisting_name')])
@@ -287,6 +294,17 @@
         res_17 = self.search(cr, uid, [('website', 'not in', False)])
         res_17.sort()
         assert res_17 == with_website
+
+-
+    Check behavior for required many2one fields
+-
+    !python {model: res.company }: |
+        company_ids = sorted(self.search(cr, uid, []))
+        # currency_id is required
+        res_101 = sorted(self.search(cr, uid, [('currency_id', 'not ilike', '')])) # get no companies
+        res_102 = sorted(self.search(cr, uid, [('currency_id', 'ilike', '')])) # get all companies
+        assert res_101 == [], 'res_101: expected %r, got %r' % ([], res_101)
+        assert res_102 == company_ids, 'res_102: expected %r, got %r' % (company_ids, res_102)
 -
     Property of the query (one2many not in False).
 -

openerp/osv/expression.py

@@ -593,23 +593,13 @@ class expression(object):
                         if operator in NEGATIVE_TERM_OPERATORS:
                             res_ids.append(False) # TODO this should not be appended if False was in 'right'
                         return (left, 'in', res_ids)
-
-                    m2o_str = False
-                    if right:
-                        if isinstance(right, basestring): # and not isinstance(field, fields.related):
-                            m2o_str = True
-                        elif isinstance(right, (list, tuple)):
-                            m2o_str = True
-                            for ele in right:
-                                if not isinstance(ele, basestring):
-                                    m2o_str = False
-                                    break
-                        if m2o_str:
-                            self.__exp[i] = _get_expression(field_obj, cr, uid, left, right, operator, context=context)
-                    elif right == []:
-                        pass # Handled by __leaf_to_sql().
-                    else: # right is False
-                        pass # Handled by __leaf_to_sql().
+                    # resolve string-based m2o criterion into IDs
+                    if isinstance(right, basestring) or \
+                            right and isinstance(right, (tuple,list)) and all(isinstance(item, basestring) for item in right):
+                        self.__exp[i] = _get_expression(field_obj, cr, uid, left, right, operator, context=context)
+                    else: 
+                        # right == [] or right == False and all other cases are handled by __leaf_to_sql()
+                        pass
 
             else:
                 # other field type

openerp/osv/orm.py

@@ -2265,7 +2265,8 @@ class BaseModel(object):
         if context is None:
             context = {}
         args = args[:]
-        if name:
+        # optimize out the default criterion of ``ilike ''`` that matches everything
+        if not (name == '' and operator == 'ilike'):
             args += [(self._rec_name, operator, name)]
         access_rights_uid = name_get_uid or user
         ids = self._search(cr, user, args, limit=limit, context=context, access_rights_uid=access_rights_uid)