[IMP] use non-literal domains to and from client, add a bunch of tests

Converted OpenERPSession's evaluations to be based on literal (dict) or nonliteral (Domain) objects. OpenERPSession will *not accept* to eval strings

bzr revid: xmo@openerp.com-20110328141920-hlp6sb173o2j6ldw

5 files changed, 123 insertions, 22 deletions

diff --git a/addons/base/controllers/main.py b/addons/base/controllers/main.py
index 6752ea8c7d7..7981c717324 100644
--- a/addons/base/controllers/main.py
+++ b/addons/base/controllers/main.py
@@ -21,7 +21,8 @@ class Xml2Json:
     # URL: http://code.google.com/p/xml2json-direct/
     @staticmethod
     def convert_to_json(s):
-        return simplejson.dumps(Xml2Json.convert_to_structure(s), sort_keys=True, indent=4)
+        return simplejson.dumps(
+            Xml2Json.convert_to_structure(s), sort_keys=True, indent=4)
 
     @staticmethod
     def convert_to_structure(s):
@@ -174,10 +175,15 @@ class Menu(openerpweb.Controller):
         actions = Values.get('action', 'tree_but_open', [('ir.ui.menu', menu_id)], False, {})
 
         for _, _, action in actions:
+            # values come from the server, we can just eval them
             action['context'] = req.session.eval_context(
                 action['context']) or {}
-            action['domain'] = req.session.eval_domain(
-                action['domain'], action['context']) or []
+
+            if isinstance(action['domain'], basestring):
+                action['domain'] = eval(
+                    action['domain'],
+                    req.session.evaluation_context(
+                        action['context'])) or []
 
         return {"action": actions}
 
diff --git a/openerpweb/nonliterals.py b/openerpweb/nonliterals.py
index b209fc9e3e4..f381536749d 100644
--- a/openerpweb/nonliterals.py
+++ b/openerpweb/nonliterals.py
@@ -6,6 +6,8 @@ can't be sent there themselves).
 """
 import binascii
 import hashlib
+import simplejson.decoder
+import simplejson.encoder
 
 __all__ = ['Domain']
 
@@ -13,6 +15,21 @@ __all__ = ['Domain']
 #: with a million hashes, according to hg@67081329d49a
 SHORT_HASH_BYTES_SIZE = 6
 
+class NonLiteralEncoder(simplejson.encoder.JSONEncoder):
+    def default(self, object):
+        if isinstance(object, Domain):
+            return {
+                '__ref': 'domain',
+                '__id': object.key
+            }
+        return super(NonLiteralEncoder, self).default(object)
+
+def non_literal_decoder(dct):
+    if '__ref' in dct:
+        if dct['__ref'] == 'domain':
+            return Domain(None, key=dct['__id'])
+    return dct
+
 class Domain(object):
     def __init__(self, session, domain_string=None, key=None):
         """ Uses session information to store the domain string and map it to a
diff --git a/openerpweb/openerpweb.py b/openerpweb/openerpweb.py
index d07df92bb36..0f8786b7b41 100644
--- a/openerpweb/openerpweb.py
+++ b/openerpweb/openerpweb.py
@@ -15,6 +15,7 @@ import cherrypy
 import cherrypy.lib.static
 import simplejson
 
+import nonliterals
 import xmlrpctimeout
 
 #----------------------------------------------------------
@@ -197,31 +198,37 @@ class OpenERPSession(object):
             current_context.update(final_context)
         return final_context
 
-    def eval_domain(self, domain_string, context=None, use_base=True):
+    def eval_domain(self, domain, context=None):
         """ Evaluates the provided domain_string using the provided context
         (merged with the session's evaluation context)
 
-        :param str domain_string: an OpenERP domain as a string, to evaluate.
+        :param domain: an OpenERP domain as a dict or as a
+                       :class:`openerpweb.nonliterals.Domain` instance
 
-                              If not a string, is returned as-is
+                       In the second case, it will be evaluated and returned.
+        :type domain: openerpweb.nonliterals.Domain
         :param dict context: the context to use in the evaluation, if any.
-        :param bool use_base: whether the base eval context (combination
-                              of the default context and the session
-                              context) should be used
         :returns: the evaluated domain
         :rtype: list
+
+        :raises: ``TypeError`` if ``domain`` is neither a dict nor a Domain
         """
-        if not isinstance(domain_string, basestring):
-            return domain_string
+        if not isinstance(domain, (list, nonliterals.Domain)):
+            raise TypeError("Domain %r is not a dict or a nonliteral Domain",
+                             domain)
+
+        if isinstance(domain, list):
+            return domain
 
         ctx = {}
-        if use_base:
-            ctx.update(self.base_eval_context)
         if context:
             ctx.update(context)
         ctx['context'] = ctx
 
-        return eval(domain_string, ctx)
+        # if the domain was unpacked from JSON, it needs the current
+        # OpenERPSession for its data retrieval
+        domain.session = self
+        return domain.evaluate(ctx)
 
     def eval_domains(self, domains, context=None):
         """ Evaluates and concatenates the provided domains using the
@@ -236,14 +243,10 @@ class OpenERPSession(object):
         :returns: the final combination of all domains in the sequence
         :rtype: list
         """
-        ctx = dict(
-            self.base_eval_context,
-            **(context or {}))
-
         final_domain = []
         for domain in domains:
             final_domain.extend(
-                self.eval_domain(domain, ctx))
+                self.eval_domain(domain, context))
         return final_domain
 
 #----------------------------------------------------------
@@ -290,9 +293,11 @@ class JsonRequest(object):
         :rtype: bytes
         '''
         if requestf:
-            request = simplejson.load(requestf)
+            request = simplejson.load(
+                requestf, object_hook=nonliterals.non_literal_decoder)
         else:
-            request = simplejson.loads(request)
+            request = simplejson.loads(
+                request, object_hook=nonliterals.non_literal_decoder)
         try:
             print "--> %s.%s %s" % (controller.__class__.__name__, method.__name__, request)
             error = None
@@ -335,7 +340,8 @@ class JsonRequest(object):
         print "<--", response
         print
 
-        content = simplejson.dumps(response)
+        content = simplejson.dumps(
+            response, cls=nonliterals.NonLiteralEncoder)
         cherrypy.response.headers['Content-Type'] = 'application/json'
         cherrypy.response.headers['Content-Length'] = len(content)
         return content
diff --git a/openerpweb/tests/test_nonliterals.py b/openerpweb/tests/test_nonliterals.py
index 9e1685721d1..4ebb50847d1 100644
--- a/openerpweb/tests/test_nonliterals.py
+++ b/openerpweb/tests/test_nonliterals.py
@@ -1,8 +1,10 @@
 # -*- coding: utf-8 -*-
 import mock
+import simplejson
 import unittest2
 
 from openerpweb.nonliterals import Domain
+import openerpweb.nonliterals
 import openerpweb.openerpweb
 
 class NonLiteralDomainTest(unittest2.TestCase):
@@ -41,3 +43,14 @@ class NonLiteralDomainTest(unittest2.TestCase):
         result = Domain(self.session, "[('a', '=', foo)]").evaluate({'foo': 3})
         self.assertEqual(
             result, [('a', '=', 3)])
+
+class NonLiteralJSON(unittest2.TestCase):
+    def setUp(self):
+        self.session = mock.Mock(spec=openerpweb.openerpweb.OpenERPSession)
+        self.session.domains_store = {}
+
+    def test_encode(self):
+        d = Domain(self.session, "some arbitrary string")
+        self.assertEqual(
+            simplejson.dumps(d, cls=openerpweb.nonliterals.NonLiteralEncoder),
+            simplejson.dumps({'__ref': 'domain', '__id': d.key}))
diff --git a/openerpweb/tests/test_session.py b/openerpweb/tests/test_session.py
index 2f982e87f7c..e7e0381cec5 100644
--- a/openerpweb/tests/test_session.py
+++ b/openerpweb/tests/test_session.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 import unittest2
+from openerpweb.nonliterals import Domain
 import openerpweb.openerpweb
 
 class TestOpenERPSession(unittest2.TestCase):
@@ -49,3 +50,61 @@ class TestOpenERPSession(unittest2.TestCase):
         self.assertEqual(
             eval('foo + 3', self.session.evaluation_context({'foo': 4})),
             7)
+
+    def test_eval_domain_typeerror(self):
+        self.assertRaises(
+            TypeError, self.session.eval_domain, "foo")
+
+    def test_eval_domain_list(self):
+        self.assertEqual(
+            self.session.eval_domain([]),
+            [])
+
+    def test_eval_nonliteral_domain(self):
+        d = Domain(self.session, "[('foo', 'is', 3)]")
+        self.assertEqual(
+            self.session.eval_domain(d),
+            [('foo', 'is', 3)])
+
+    def test_eval_nonliteral_domain_bykey(self):
+        key = Domain(
+            self.session, "[('foo', 'is', 3)]").key
+
+        d = Domain(None, key=key)
+        self.assertEqual(
+            self.session.eval_domain(d),
+            [('foo', 'is', 3)])
+
+    def test_eval_empty_domains(self):
+        self.assertEqual(
+            self.session.eval_domains([]),
+            [])
+
+    def test_eval_literal_domains(self):
+        domains = [
+            [('a', 'is', 3)],
+            [('b', 'ilike', 'foo')],
+            ['|',
+             ('c', '=', False),
+             ('c', 'in', ['a', 'b', 'c'])]
+        ]
+        self.assertEqual(
+            self.session.eval_domains(domains),
+            [
+                ('a', 'is', 3),
+                ('b', 'ilike', 'foo'),
+                '|',
+                ('c', '=', False),
+                ('c', 'in', ['a', 'b', 'c'])
+            ])
+    def test_eval_nonliteral_domains(self):
+        domains = [
+            Domain(self.session, "[('uid', '=', uid)]"),
+            Domain(self.session,
+                   "['|', ('date', '<', current_date),"
+                        " ('date', '>', current_date)]")]
+        self.assertEqual(
+            self.session.eval_domains(domains),
+            [('uid', '=', -1),
+             '|', ('date', '<', '1945-08-05'), ('date', '>', '1945-08-05')]
+        )