[FIX] expression: improve o2m with m2o integer

We sometimes use in Odoo a One2many field with an inverse Integer
field instead of a usual Many2one.

This allow for example in several instances to have a "Many2one" which
can be reference from several models, eg:

Model Ranking:
	name = String field
	res_id = Integer field
	res_model = String field

Model Toy:
	rank = One2many [inverse: Ranking -> res_id]
                        [domain: res_model == Toy]

Model Tool:
	rank = One2many [inverse: Ranking -> res_id]
                        [domain: res_model == Tool]

This enable us to have a shared feature between otherwise unrelated models.

But there was several issue when searching on these One2many:

1) if the Integer Many2one was not stored (eg. it came from an inherits) on
   the searched model, this could lead to an error.

2) when we searched:

   - by IDs (rank in ['55']) with at least one id not respecting the domain
   - by IDs with a negative operator
   - with a negative operator on unfound string (rank != "no rank has this")
   - with a false value (rank = False)

   we would not apply the One2many domain (eg. res_model == Toy) and thus
   possibly getting Toy 3 errenously because a Tool 3 was found without
   the domain being applied.

This fix modify the search on One2many and for:

1. if the inverse is an Integer not stored field instead of Many2one
   manage it.

2. if the field is an Integer field instead of Many2one and there is a
   domain on the One2many: apply the domain on the inverse model found.

So only some search on One2many with a domain whose inverse field is an
Integer could be impacted.

This would also be nice to have for all One2many with a domain but the
probability of it being useful versus risk for performance is not judged
high enough.

opw-710508
This commit is contained in:
Nicolas Lempereur
2017-03-16 11:57:04 +01:00
parent 27f656a23e
commit cc1c15327c
4 changed files with 118 additions and 11 deletions
@@ -5,6 +5,9 @@ access_message,test_new_api_message,test_new_api.model_test_new_api_message,,1,1
access_emailmessage,test_new_api_emailmessage,test_new_api.model_test_new_api_emailmessage,,1,1,1,1
access_multi,test_new_api_multi,test_new_api.model_test_new_api_multi,,1,1,1,1
access_multi_line,test_new_api_multi_line,test_new_api.model_test_new_api_multi_line,,1,1,1,1
access_creativework_edition,test_new_api_creativework_edition,model_test_new_api_creativework_edition,,1,1,1,1
access_creativework_book,test_new_api_creativework_book,model_test_new_api_creativework_book,,1,1,1,1
access_creativework_movie,test_new_api_creativework_movie,model_test_new_api_creativework_movie,,1,1,1,1
access_mixed,test_new_api_mixed,test_new_api.model_test_new_api_mixed,,1,1,1,1
access_domain_bool,access_domain_bool,model_domain_bool,,1,1,1,1
access_test_new_api_foo,access_test_new_api_foo,model_test_new_api_foo,,1,1,1,1
1 id name model_id:id group_id:id perm_read perm_write perm_create perm_unlink
5 access_emailmessage test_new_api_emailmessage test_new_api.model_test_new_api_emailmessage 1 1 1 1
6 access_multi test_new_api_multi test_new_api.model_test_new_api_multi 1 1 1 1
7 access_multi_line test_new_api_multi_line test_new_api.model_test_new_api_multi_line 1 1 1 1
8 access_creativework_edition test_new_api_creativework_edition model_test_new_api_creativework_edition 1 1 1 1
9 access_creativework_book test_new_api_creativework_book model_test_new_api_creativework_book 1 1 1 1
10 access_creativework_movie test_new_api_creativework_movie model_test_new_api_creativework_movie 1 1 1 1
11 access_mixed test_new_api_mixed test_new_api.model_test_new_api_mixed 1 1 1 1
12 access_domain_bool access_domain_bool model_domain_bool 1 1 1 1
13 access_test_new_api_foo access_test_new_api_foo model_test_new_api_foo 1 1 1 1
+27
View File
@@ -199,6 +199,33 @@ class MultiLine(models.Model):
partner = fields.Many2one('res.partner')
class Edition(models.Model):
_name = 'test_new_api.creativework.edition'
name = fields.Char()
res_id = fields.Integer(required=True)
res_model_id = fields.Many2one('ir.model', required=True)
res_model = fields.Char(related='res_model_id.model', store=True)
class Book(models.Model):
_name = 'test_new_api.creativework.book'
name = fields.Char()
editions = fields.One2many(
'test_new_api.creativework.edition', 'res_id', domain=[('res_model', '=', _name)]
)
class Movie(models.Model):
_name = 'test_new_api.creativework.movie'
name = fields.Char()
editions = fields.One2many(
'test_new_api.creativework.edition', 'res_id', domain=[('res_model', '=', _name)]
)
class MixedModel(models.Model):
_name = 'test_new_api.mixed'
@@ -10,6 +10,36 @@ class One2manyCase(TransactionCase):
"name": "What is up?"
})
# data for One2many with inverse field Integer
self.Edition = self.env["test_new_api.creativework.edition"]
self.Book = self.env["test_new_api.creativework.book"]
self.Movie = self.env["test_new_api.creativework.movie"]
book_model_id = self.env['ir.model'].search([('model', '=', self.Book._name)]).id
movie_model_id = self.env['ir.model'].search([('model', '=', self.Movie._name)]).id
books_data = (
('Imaginary book', ()),
('Another imaginary book', ()),
('Nineteen Eighty Four', ('First edition', 'Fourth Edition'))
)
movies_data = (
('The Gold Rush', ('1925 (silent)', '1942')),
('Imaginary movie', ()),
('Another imaginary movie', ())
)
for name, editions in books_data:
book_id = self.Book.create({'name': name}).id
for edition in editions:
self.Edition.create({'res_model_id': book_model_id, 'name': edition, 'res_id': book_id})
for name, editions in movies_data:
movie_id = self.Movie.create({'name': name}).id
for edition in editions:
self.Edition.create({'res_model_id': movie_model_id, 'name': edition, 'res_id': movie_id})
def operations(self):
"""Run operations on o2m fields to check all works fine."""
# Check the lines first
@@ -63,3 +93,40 @@ class One2manyCase(TransactionCase):
"""Check lines created with RPC style and added in one step"""
self.multi.lines = [(0, 0, {'name': str(name)}) for name in range(10)]
self.operations()
def test_many2one_integer(self):
"""Test several models one2many with same inverse Integer field"""
# utility function to convert records to tuples with id,name
t = lambda records: records.mapped(lambda r: (r.id, r.name))
books = self.Book.search([])
books_with_edition = books.filtered(lambda r: r.editions)
movies = self.Movie.search([])
movies_without_edition = movies.filtered(lambda r: not r.editions)
movies_with_edition = movies.filtered(lambda r: r.editions)
movie_editions = movies_with_edition.mapped('editions')
one_movie_edition = movie_editions[0]
res_movies_without_edition = self.Movie.search([('editions', '=', False)])
self.assertItemsEqual(t(res_movies_without_edition), t(movies_without_edition))
res_movies_with_edition = self.Movie.search([('editions', '!=', False)])
self.assertItemsEqual(t(res_movies_with_edition), t(movies_with_edition))
res_books_with_movie_edition = self.Book.search([('editions', 'in', movie_editions.ids)])
self.assertFalse(t(res_books_with_movie_edition))
res_books_without_movie_edition = self.Book.search([('editions', 'not in', movie_editions.ids)])
self.assertItemsEqual(t(res_books_without_movie_edition), t(books_with_edition))
res_books_without_one_movie_edition = self.Book.search([('editions', 'not in', movie_editions[:1].ids)])
self.assertItemsEqual(t(res_books_without_one_movie_edition), t(books_with_edition))
res_books_with_one_movie_edition_name = self.Book.search([('editions', '=', movie_editions[:1].name)])
self.assertFalse(t(res_books_with_one_movie_edition_name))
res_books_without_one_movie_edition_name = self.Book.search([('editions', '!=', movie_editions[:1].name)])
self.assertItemsEqual(t(res_books_without_one_movie_edition_name), t(books_with_edition))
res_movies_not_of_edition_name = self.Movie.search([('editions', '!=', one_movie_edition.name)])
self.assertItemsEqual(t(res_movies_not_of_edition_name), t(movies.filtered(lambda r: one_movie_edition not in r.editions)))
+21 -11
View File
@@ -920,19 +920,23 @@ class expression(object):
elif field.type == 'one2many':
call_null = True
domain = field.domain
if callable(domain):
domain = domain(model)
is_integer_m2o = comodel._fields[field.inverse_name].type == 'integer'
if right is not False:
if isinstance(right, basestring):
op = {'!=': '=', 'not like': 'like', 'not ilike': 'ilike'}.get(operator, operator)
domain = field.domain
if callable(domain):
domain = domain(model)
ids2 = [x[0] for x in comodel.name_search(right, domain or [], op, limit=None)]
if ids2:
operator = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in'
elif isinstance(right, collections.Iterable):
ids2 = right
else:
ids2 = [right]
if isinstance(right, collections.Iterable):
ids2 = right
else:
ids2 = [right]
if ids2 and is_integer_m2o and domain:
ids2 = comodel.search([('id', 'in', ids2)] + domain).ids
if not ids2:
if operator in ['like', 'ilike', 'in', '=']:
@@ -945,7 +949,9 @@ class expression(object):
ids1 = select_from_where(cr, field.inverse_name, comodel._table, 'id', ids2, operator)
else:
recs = comodel.browse(ids2).sudo().with_context(prefetch_fields=False)
ids1 = recs.mapped(field.inverse_name).ids
ids1 = recs.mapped(field.inverse_name)
if not is_integer_m2o:
ids1 = ids1.ids
if ids1:
call_null = False
o2m_op = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in'
@@ -958,12 +964,16 @@ class expression(object):
if call_null:
o2m_op = 'in' if operator in NEGATIVE_TERM_OPERATORS else 'not in'
# determine ids from field.inverse_name
if comodel._fields[field.inverse_name].store:
if comodel._fields[field.inverse_name].store and not (is_integer_m2o and domain):
ids1 = select_distinct_from_where_not_null(cr, field.inverse_name, comodel._table)
else:
domain = [(field.inverse_name, '!=', False)]
recs = comodel.search(domain).sudo().with_context(prefetch_fields=False)
ids1 = recs.mapped(field.inverse_name).ids
comodel_domain = [(field.inverse_name, '!=', False)]
if is_integer_m2o and domain:
comodel_domain += domain
recs = comodel.search(comodel_domain).sudo().with_context(prefetch_fields=False)
ids1 = recs.mapped(field.inverse_name)
if not is_integer_m2o:
ids1 = ids1.ids
push(create_substitution_leaf(leaf, ('id', o2m_op, ids1), model))
elif field.type == 'many2many':