[IMP] models: do not use compute methods to determine default values anymore

Compute methods could give results that should not be considered as default
values.  For instance, a related field usually defaults to a null value, which
is then set to the field with its inverse method by create().  This may violate
a non-null constraint if the original field is required.  Therefore, compute
methods are no longer used to determine default values.

5 files changed, 52 insertions, 69 deletions

diff --git a/openerp/addons/base/ir/ir_values.py b/openerp/addons/base/ir/ir_values.py
index 4720f601f49..075612c6474 100644
--- a/openerp/addons/base/ir/ir_values.py
+++ b/openerp/addons/base/ir/ir_values.py
@@ -335,7 +335,7 @@ class ir_values(osv.osv):
                 (row['id'], row['name'], pickle.loads(row['value'].encode('utf-8'))))
         return defaults.values()
 
-    # use ormcache: this is called a lot by BaseModel.add_default_value()!
+    # use ormcache: this is called a lot by BaseModel.default_get()!
     @tools.ormcache(skiparg=2)
     def get_defaults_dict(self, cr, uid, model, condition=False):
         """ Returns a dictionary mapping field names with their corresponding
diff --git a/openerp/addons/test_new_api/models.py b/openerp/addons/test_new_api/models.py
index 3b152750b8d..6a2233427f6 100644
--- a/openerp/addons/test_new_api/models.py
+++ b/openerp/addons/test_new_api/models.py
@@ -117,7 +117,7 @@ class Message(models.Model):
     @api.one
     @api.depends('author.name', 'discussion.name')
     def _compute_name(self):
-        self.name = "[%s] %s" % (self.discussion.name or '', self.author.name)
+        self.name = "[%s] %s" % (self.discussion.name or '', self.author.name or '')
 
     @api.one
     @api.depends('author.name', 'discussion.name', 'body')
diff --git a/openerp/addons/test_new_api/tests/test_new_fields.py b/openerp/addons/test_new_api/tests/test_new_fields.py
index 14ff358cc90..74f85fa6d68 100644
--- a/openerp/addons/test_new_api/tests/test_new_fields.py
+++ b/openerp/addons/test_new_api/tests/test_new_fields.py
@@ -349,20 +349,18 @@ class TestNewFields(common.TransactionCase):
         message.body = BODY = "May the Force be with you."
         self.assertEqual(message.discussion, discussion)
         self.assertEqual(message.body, BODY)
-
+        self.assertFalse(message.author)
         self.assertNotIn(message, discussion.messages)
 
         # check computed values of fields
-        user = self.env.user
-        self.assertEqual(message.author, user)
-        self.assertEqual(message.name, "[%s] %s" % (discussion.name, user.name))
+        self.assertEqual(message.name, "[%s] %s" % (discussion.name, ''))
         self.assertEqual(message.size, len(BODY))
 
     def test_41_defaults(self):
         """ test default values. """
         fields = ['discussion', 'body', 'author', 'size']
         defaults = self.env['test_new_api.message'].default_get(fields)
-        self.assertEqual(defaults, {'author': self.env.uid, 'size': 0})
+        self.assertEqual(defaults, {'author': self.env.uid})
 
         defaults = self.env['test_new_api.mixed'].default_get(['number'])
         self.assertEqual(defaults, {'number': 3.14})
diff --git a/openerp/fields.py b/openerp/fields.py
index e747d557d9a..748576c6cfd 100644
--- a/openerp/fields.py
+++ b/openerp/fields.py
@@ -753,8 +753,8 @@ class Field(object):
             # normal record -> read or compute value for this field
             self.determine_value(record)
         else:
-            # new record -> compute default value for this field
-            record.add_default_value(self)
+            # draft record -> compute the value or let it be null
+            self.determine_draft_value(record)
 
         # the result should be in cache now
         return record._cache[self]
@@ -861,12 +861,9 @@ class Field(object):
             # this is a non-stored non-computed field
             record._cache[self] = self.null(env)
 
-    def determine_default(self, record):
-        """ determine the default value of field `self` on `record` """
-        if self.default:
-            value = self.default(record)
-            record._cache[self] = self.convert_to_cache(value, record)
-        elif self.compute:
+    def determine_draft_value(self, record):
+        """ Determine the value of `self` for the given draft `record`. """
+        if self.compute:
             self._compute_value(record)
         else:
             record._cache[self] = SpecialValue(self.null(record.env))
@@ -1470,15 +1467,6 @@ class Many2one(_Relational):
     def convert_to_display_name(self, value):
         return ustr(value.display_name)
 
-    def determine_default(self, record):
-        super(Many2one, self).determine_default(record)
-        if self.delegate:
-            # special case: fields that implement inheritance between models
-            value = record[self.name]
-            if not value:
-                # the default value cannot be null, use a new record instead
-                record[self.name] = record.env[self.comodel_name].new()
-
 
 class UnionUpdate(SpecialValue):
     """ Placeholder for a value update; when this value is taken from the cache,
diff --git a/openerp/models.py b/openerp/models.py
index 1457a4beecf..a4597a21299 100644
--- a/openerp/models.py
+++ b/openerp/models.py
@@ -1305,7 +1305,8 @@ class BaseModel(object):
                 except Exception, e:
                     raise ValidationError("Error while validating constraint\n\n%s" % tools.ustr(e))
 
-    def default_get(self, cr, uid, fields_list, context=None):
+    @api.model
+    def default_get(self, fields_list):
         """ default_get(fields) -> default_values
 
         Return default values for the fields in `fields_list`. Default
@@ -1314,60 +1315,56 @@ class BaseModel(object):
 
         :param fields_list: a list of field names
         :return: a dictionary mapping each field name to its corresponding
-            default value; the keys of the dictionary are the fields in
-            `fields_list` that have a default value different from ``False``.
+            default value, if it has one.
 
-        This method should not be overridden. In order to change the
-        mechanism for determining default values, you should override method
-        :meth:`add_default_value` instead.
         """
         # trigger view init hook
-        self.view_init(cr, uid, fields_list, context)
+        self.view_init(fields_list)
+
+        defaults = {}
+        parent_fields = defaultdict(list)
 
-        # use a new record to determine default values; evaluate fields on the
-        # new record and put default values in result
-        record = self.new(cr, uid, {}, context=context)
-        result = {}
         for name in fields_list:
-            if name in self._fields:
-                value = record[name]
-                if name in record._cache:
-                    result[name] = value        # it really is a default value
+            # 1. look up context
+            key = 'default_' + name
+            if key in self._context:
+                defaults[name] = self._context[key]
+                continue
 
-        # convert default values to the expected format
-        result = self._convert_to_write(result)
-        return result
+            # 2. look up ir_values
+            #    Note: performance is good, because get_defaults_dict is cached!
+            ir_values_dict = self.env['ir.values'].get_defaults_dict(self._name)
+            if name in ir_values_dict:
+                defaults[name] = ir_values_dict[name]
+                continue
 
-    def add_default_value(self, field):
-        """ Set the default value of `field` to the new record `self`.
-            The value must be assigned to `self`.
-        """
-        assert not self.id, "Expected new record: %s" % self
-        cr, uid, context = self.env.args
-        name = field.name
+            field = self._fields.get(name)
 
-        # 1. look up context
-        key = 'default_' + name
-        if key in context:
-            self[name] = context[key]
-            return
+            # 3. look up property fields
+            #    TODO: get rid of this one
+            if field and field.company_dependent:
+                defaults[name] = self.env['ir.property'].get(name, self._name)
+                continue
 
-        # 2. look up ir_values
-        #    Note: performance is good, because get_defaults_dict is cached!
-        ir_values_dict = self.env['ir.values'].get_defaults_dict(self._name)
-        if name in ir_values_dict:
-            self[name] = ir_values_dict[name]
-            return
+            # 4. look up field.default
+            if field and field.default:
+                defaults[name] = field.default(self)
+                continue
 
-        # 3. look up property fields
-        #    TODO: get rid of this one
-        column = self._columns.get(name)
-        if isinstance(column, fields.property):
-            self[name] = self.env['ir.property'].get(name, self._name)
-            return
+            # 5. delegate to parent model
+            if field and field.inherited:
+                field = field.related_field
+                parent_fields[field.model_name].append(field.name)
+
+        # convert default values to the right format
+        defaults = self._convert_to_cache(defaults, validate=False)
+        defaults = self._convert_to_write(defaults)
+
+        # add default values for inherited fields
+        for model, names in parent_fields.iteritems():
+            defaults.update(self.env[model].default_get(names))
 
-        # 4. delegate to field
-        field.determine_default(self)
+        return defaults
 
     def fields_get_keys(self, cr, user, context=None):
         res = self._columns.keys()
@@ -5260,7 +5257,7 @@ class BaseModel(object):
 
     #
     # New records - represent records that do not exist in the database yet;
-    # they are used to compute default values and perform onchanges.
+    # they are used to perform onchanges.
     #
 
     @api.model