Merge lp:~camptocamp/openobject-addons/7.0-bugfix-1189480-with-perf-mdh into lp:openobject-addons/7.0

Proposed by Matthieu Dietrich @ camptocamp
Status: Needs review
Proposed branch: lp:~camptocamp/openobject-addons/7.0-bugfix-1189480-with-perf-mdh
Merge into: lp:openobject-addons/7.0
Diff against target: 330 lines (+243/-20)
3 files modified
project/__openerp__.py (+1/-0)
project/project.py (+68/-20)
project/test/hours_process.yml (+174/-0)
To merge this branch: bzr merge lp:~camptocamp/openobject-addons/7.0-bugfix-1189480-with-perf-mdh
Reviewer Review Type Date Requested Status
Matthieu Dietrich @ camptocamp (community) Needs Resubmitting
OpenERP Core Team Pending
Review via email: mp+197326@code.launchpad.net

This proposal supersedes a proposal from 2013-11-29.

Description of the change

The quick fix proposed in lp:~camptocamp/openobject-addons/7.0-fixes-bug-1189480 works, but the multiple read() severely impact performance. This solution uses a WITH RECURSIVE SQL statement in order to 1) avoid any extraneous read() 2) get all the information in one request.

I also added a YAML test to test both this new method and a hierarchical computation, since all YAML tests in addons are for a single level of projects.

To post a comment you must log in.
Revision history for this message
Matthieu Dietrich @ camptocamp (mdietrich-c2c) wrote :

Modified the code to be PEP-8 compliant + a better check for equality on float numbers.

review: Needs Resubmitting

Unmerged revisions

9665. By Matthieu Dietrich @ camptocamp on 2013-12-02

[FIX] PEP8-compliance + better check for float numbers

9664. By Matthieu Dietrich @ camptocamp on 2013-11-29

[IMP] performance improvement on progress rate calculation in projects

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'project/__openerp__.py'
2--- project/__openerp__.py 2013-01-31 18:41:41 +0000
3+++ project/__openerp__.py 2013-12-02 08:31:51 +0000
4@@ -82,6 +82,7 @@
5 'test/project_demo.yml',
6 'test/project_process.yml',
7 'test/task_process.yml',
8+ 'test/hours_process.yml',
9 ],
10 'installable': True,
11 'auto_install': False,
12
13=== modified file 'project/project.py'
14--- project/project.py 2013-11-22 16:55:04 +0000
15+++ project/project.py 2013-12-02 08:31:51 +0000
16@@ -135,6 +135,7 @@
17 res.update(ids)
18 return list(res)
19
20+ # Deprecated; the _progress_rate method does not use this anymore
21 def _get_project_and_children(self, cr, uid, ids, context=None):
22 """ retrieve all children projects of project ids;
23 return a dictionary mapping each project to its parent project (or None)
24@@ -154,28 +155,75 @@
25 return res
26
27 def _progress_rate(self, cr, uid, ids, names, arg, context=None):
28- child_parent = self._get_project_and_children(cr, uid, ids, context)
29 # compute planned_hours, total_hours, effective_hours specific to each project
30+ # How this works: the WITH RECURSIVE statement will create an union line
31+ # for each parent project with the hours of each child project; the final
32+ # SUM(...) ensures we get a total of hours by project.
33 cr.execute("""
34- SELECT project_id, COALESCE(SUM(planned_hours), 0.0),
35- COALESCE(SUM(total_hours), 0.0), COALESCE(SUM(effective_hours), 0.0)
36- FROM project_task WHERE project_id IN %s AND state <> 'cancelled'
37- GROUP BY project_id
38- """, (tuple(child_parent.keys()),))
39+ WITH RECURSIVE recur_table(project_id,
40+ parent_id,
41+ planned_hours,
42+ total_hours,
43+ effective_hours) AS (
44+ SELECT project.id,
45+ parent.id,
46+ COALESCE(task.planned, 0.0),
47+ COALESCE(task.total, 0.0),
48+ COALESCE(task.effective, 0.0)
49+ FROM project_project project
50+ LEFT JOIN account_analytic_account account
51+ ON project.analytic_account_id = account.id
52+ LEFT JOIN project_project parent
53+ ON parent.analytic_account_id = account.parent_id
54+ LEFT JOIN (SELECT project_id,
55+ SUM(planned_hours) as planned,
56+ SUM(total_hours) as total,
57+ SUM(effective_hours) as effective
58+ FROM project_task
59+ WHERE state <> 'cancelled'
60+ GROUP BY project_id) AS task
61+ ON project.id = task.project_id
62+ UNION ALL
63+ SELECT project.id,
64+ parent.id,
65+ recur_table.planned_hours,
66+ recur_table.total_hours,
67+ recur_table.effective_hours
68+ FROM project_project project
69+ LEFT JOIN account_analytic_account account
70+ ON project.analytic_account_id = account.id
71+ LEFT JOIN project_project parent
72+ ON parent.analytic_account_id = account.parent_id
73+ LEFT JOIN (SELECT project_id,
74+ SUM(planned_hours) as planned,
75+ SUM(total_hours) as total,
76+ SUM(effective_hours) as effective
77+ FROM project_task
78+ WHERE state <> 'cancelled'
79+ GROUP BY project_id) AS task
80+ ON project.id = task.project_id
81+ JOIN recur_table ON project.id = recur_table.parent_id
82+ )
83+ SELECT project_id,
84+ SUM(planned_hours),
85+ SUM(total_hours),
86+ SUM(effective_hours)
87+ FROM recur_table
88+ WHERE project_id IN %s
89+ GROUP BY project_id
90+ """, (tuple(ids),))
91 # aggregate results into res
92- res = dict([(id, {'planned_hours':0.0,'total_hours':0.0,'effective_hours':0.0}) for id in ids])
93- for id, planned, total, effective in cr.fetchall():
94- # add the values specific to id to all parent projects of id in the result
95- while id:
96- if id in ids:
97- res[id]['planned_hours'] += planned
98- res[id]['total_hours'] += total
99- res[id]['effective_hours'] += effective
100- id = child_parent[id]
101+ res = dict([(result_line[0], {'planned_hours': result_line[1],
102+ 'total_hours': result_line[2],
103+ 'effective_hours': result_line[3]})
104+ for result_line in cr.fetchall()])
105 # compute progress rates
106 for id in ids:
107 if res[id]['total_hours']:
108- res[id]['progress_rate'] = round(100.0 * res[id]['effective_hours'] / res[id]['total_hours'], 2)
109+ res[id]['progress_rate'] = round(100.0 *
110+ res[id]['effective_hours'] /
111+ res[id]['total_hours'],
112+ 2)
113 else:
114 res[id]['progress_rate'] = 0.0
115 return res
116@@ -192,7 +240,7 @@
117 res = super(project, self).unlink(cr, uid, ids, context=context)
118 mail_alias.unlink(cr, uid, alias_ids, context=context)
119 return res
120-
121+
122 def _get_attached_docs(self, cr, uid, ids, field_name, arg, context):
123 res = {}
124 attachment = self.pool.get('ir.attachment')
125@@ -203,7 +251,7 @@
126 task_attachments = attachment.search(cr, uid, [('res_model', '=', 'project.task'), ('res_id', 'in', task_ids)], context=context, count=True)
127 res[id] = (project_attachments or 0) + (task_attachments or 0)
128 return res
129-
130+
131 def _task_count(self, cr, uid, ids, field_name, arg, context=None):
132 if context is None:
133 context = {}
134@@ -228,7 +276,7 @@
135 def attachment_tree_view(self, cr, uid, ids, context):
136 task_ids = self.pool.get('project.task').search(cr, uid, [('project_id', 'in', ids)])
137 domain = [
138- '|',
139+ '|',
140 '&', ('res_model', '=', 'project.project'), ('res_id', 'in', ids),
141 '&', ('res_model', '=', 'project.task'), ('res_id', 'in', task_ids)
142 ]
143@@ -736,7 +784,7 @@
144 new_name = _("%s (copy)") % (default.get('name', ''))
145 default.update({'name':new_name})
146 return super(task, self).copy_data(cr, uid, id, default, context)
147-
148+
149 def copy(self, cr, uid, id, default=None, context=None):
150 if context is None:
151 context = {}
152
153=== added file 'project/test/hours_process.yml'
154--- project/test/hours_process.yml 1970-01-01 00:00:00 +0000
155+++ project/test/hours_process.yml 2013-12-02 08:31:51 +0000
156@@ -0,0 +1,174 @@
157+-
158+ First, create the analytic accounts
159+-
160+ !record {model: account.analytic.account, id: parent_project_account}:
161+ name: Parent Account
162+ code: PAR
163+ type: view
164+-
165+ !record {model: account.analytic.account, id: child1_project_account}:
166+ name: Child Account 1
167+ code: C1
168+ type: view
169+-
170+ I create a main project
171+-
172+ !record {model: project.project, id: project_project_parent}:
173+ company_id: base.main_company
174+ name: Parent Project
175+ user_id: base.user_demo
176+ parent_id: all_projects_account
177+ analytic_account_id: parent_project_account
178+ alias_model: project.task
179+-
180+ A first child project
181+-
182+ !record {model: project.project, id: project_project_child_1}:
183+ company_id: base.main_company
184+ name: Child Project 1
185+ user_id: base.user_demo
186+ parent_id: parent_project_account
187+ analytic_account_id: child1_project_account
188+ alias_model: project.task
189+-
190+ A second child project
191+-
192+ !record {model: project.project, id: project_project_child_2}:
193+ company_id: base.main_company
194+ name: Child Project 2
195+ user_id: base.user_demo
196+ parent_id: parent_project_account
197+ alias_model: project.task
198+-
199+ A child of the first child project
200+-
201+ !record {model: project.project, id: project_project_child_1_1}:
202+ company_id: base.main_company
203+ name: Child Project 1 1
204+ user_id: base.user_demo
205+ parent_id: child1_project_account
206+ alias_model: project.task
207+-
208+ A task for the main project, with 20 hours planned and 20 hours remaining
209+-
210+ !record {model: project.task, id: project_task_parent_project}:
211+ date_start: !eval time.strftime('%Y-05-%d %H:%M:%S')
212+ name: Task parent project
213+ planned_hours: 20.0
214+ remaining_hours: 20.0
215+ project_id: project_project_parent
216+ user_id: base.user_demo
217+-
218+ I test the hours
219+-
220+ !python {model: project.project}: |
221+ project = self.browse(cr, uid, ref("project_project_parent"))
222+ assert abs(project.planned_hours - 20.0) < 10**-4, "Planned hours are not correct! 20.0 != %s" % project.planned_hours
223+ assert abs(project.total_hours - 20.0) < 10**-4, "Total hours are not correct! 20.0 != %s" % project.total_hours
224+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
225+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
226+-
227+ A task for the first child project, with 30 hours planned and 30 hours remaining
228+-
229+ !record {model: project.task, id: project_task_child_project_1}:
230+ date_start: !eval time.strftime('%Y-05-%d %H:%M:%S')
231+ name: Task child project 1
232+ planned_hours: 30.0
233+ remaining_hours: 30.0
234+ project_id: project_project_child_1
235+ user_id: base.user_demo
236+-
237+ I test the hours
238+-
239+ !python {model: project.project}: |
240+ project = self.browse(cr, uid, ref("project_project_parent"))
241+ assert abs(project.planned_hours - 50.0) < 10**-4, "Planned hours are not correct! 50.0 != %s" % project.planned_hours
242+ assert abs(project.total_hours - 50.0) < 10**-4, "Total hours are not correct! 50.0 != %s" % project.total_hours
243+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
244+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
245+ project = self.browse(cr, uid, ref("project_project_child_1"))
246+ assert abs(project.planned_hours - 30.0) < 10**-4, "Planned hours are not correct! 30.0 != %s" % project.planned_hours
247+ assert abs(project.total_hours - 30.0) < 10**-4, "Total hours are not correct! 30.0 != %s" % project.total_hours
248+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
249+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
250+-
251+ A task for the second child project, with 40 hours planned and 10 hours remaining
252+-
253+ !record {model: project.task, id: project_task_child_project_2}:
254+ date_start: !eval time.strftime('%Y-05-%d %H:%M:%S')
255+ name: Task child project 2
256+ planned_hours: 40.0
257+ remaining_hours: 10.0
258+ project_id: project_project_child_2
259+ user_id: base.user_demo
260+-
261+ I test the hours
262+-
263+ !python {model: project.project}: |
264+ project = self.browse(cr, uid, ref("project_project_parent"))
265+ assert abs(project.planned_hours - 90.0) < 10**-4, "Planned hours are not correct! 90.0 != %s" % project.planned_hours
266+ assert abs(project.total_hours - 60.0) < 10**-4, "Total hours are not correct! 60.0 != %s" % project.total_hours
267+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
268+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
269+ project = self.browse(cr, uid, ref("project_project_child_2"))
270+ assert abs(project.planned_hours - 40.0) < 10**-4, "Planned hours are not correct! 40.0 != %s" % project.planned_hours
271+ assert abs(project.total_hours - 10.0) < 10**-4, "Total hours are not correct! 10.0 != %s" % project.total_hours
272+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
273+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rat
274+-
275+ A task for the child child project, with 50 hours planned and 50 hours remaining
276+-
277+ !record {model: project.task, id: project_task_child_project_1_1}:
278+ date_start: !eval time.strftime('%Y-05-%d %H:%M:%S')
279+ name: Task child project 1 1
280+ planned_hours: 50.0
281+ remaining_hours: 50.0
282+ project_id: project_project_child_1_1
283+ user_id: base.user_demo
284+-
285+ I test the hours
286+-
287+ !python {model: project.project}: |
288+ project = self.browse(cr, uid, ref("project_project_parent"))
289+ assert abs(project.planned_hours - 140.0) < 10**-4, "Planned hours are not correct! 140.0 != %s" % project.planned_hours
290+ assert abs(project.total_hours - 110.0) < 10**-4, "Total hours are not correct! 110.0 != %s" % project.total_hours
291+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
292+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
293+ project = self.browse(cr, uid, ref("project_project_child_1"))
294+ assert abs(project.planned_hours - 80.0) < 10**-4, "Planned hours are not correct! 80.0 != %s" % project.planned_hours
295+ assert abs(project.total_hours - 80.0) < 10**-4, "Total hours are not correct! 80.0 != %s" % project.total_hours
296+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
297+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
298+ project = self.browse(cr, uid, ref("project_project_child_1_1"))
299+ assert abs(project.planned_hours - 50.0) < 10**-4, "Planned hours are not correct! 50.0 != %s" % project.planned_hours
300+ assert abs(project.total_hours - 50.0) < 10**-4, "Total hours are not correct! 50.0 != %s" % project.total_hours
301+ assert abs(project.effective_hours - 0.0) < 10**-4, "Effective hours are not correct! 0.0 != %s" % project.effective_hours
302+ assert abs(project.progress_rate - 0.0) < 10**-4, "Progress rate is not correct! 0.0 != %s" % project.progress_rate
303+-
304+ I create a task work on child 1 task
305+-
306+ !record {model: project.task.work, id: project_child_1_work}:
307+ name: test work
308+ hours: 10.0
309+ task_id: project_task_child_project_1
310+ user_id: base.user_demo
311+-
312+ and refresh the task to account for it
313+-
314+ !record {model: project.task, id: project_task_child_project_1}:
315+ planned_hours: 20.0
316+-
317+ I test the hours
318+-
319+ !python {model: project.project}: |
320+ project = self.browse(cr, uid, ref("project_project_parent"))
321+ assert abs(project.planned_hours - 130.0) < 10**-4, "Planned hours are not correct! 130.0 != %s" % project.planned_hours
322+ assert abs(project.total_hours - 110.0) < 10**-4, "Total hours are not correct! 110.0 != %s" % project.total_hours
323+ assert abs(project.effective_hours - 10.0) < 10**-4, "Effective hours are not correct! 10.0 != %s" % project.effective_hours
324+ assert abs(project.progress_rate - 9.09) < 10**-4, "Progress rate is not correct! 9.09 != %s" % project.progress_rate
325+ project = self.browse(cr, uid, ref("project_project_child_1"))
326+ assert abs(project.planned_hours - 70.0) < 10**-4, "Planned hours are not correct! 70.0 != %s" % project.planned_hours
327+ assert abs(project.total_hours - 80.0) < 10**-4, "Total hours are not correct! 80.0 != %s" % project.total_hours
328+ assert abs(project.effective_hours - 10.0) < 10**-4, "Effective hours are not correct! 10.0 != %s" % project.effective_hours
329+ assert abs(project.progress_rate - 12.5) < 10**-4, "Progress rate is not correct! 12.5 != %s" % project.progress_rate
330+