From 3eec4d6a69ff76cbf750b960dc8db2eb5e0a45f9 Mon Sep 17 00:00:00 2001 From: Yannick Tivisse Date: Tue, 19 May 2020 09:18:00 +0000 Subject: [PATCH] [FW][MERGE] resource,calendar,hr_holidays: Improve the performances TL;DR ===== Improve performances with nearly a factor of 2. Use case, call action_validate on hr.leave for 100 employees - 1987 requests -> 844 requests - 1300 ms -> 700 ms Purpose ======= The first purpose of this commit is to add a test ensuring the number of request while creating a company leave for 100 employees, if 15 of them already have a leave during that period. It includes, the mass leave generation, and the conflicts resolutions. (Cancelling/Splitting the already existing one and adapting the dates accordingly). The second one is to reduce the number of request for this test. In term of requests, currently we have: - 5154 requests without bypassing the mail tracking + the activities management - 1987 requests when bypassing the mail post-process (this is the current value, the bypassing was already done several month ago) - 844 requests with all the optimization done in resource/calendar/hr_holidays In terms of execution time, we have a reduction from +- 1300 ms to call the method action_validate to +- 700 ms As the performances issues severity increases with the number of leaves to create and the real time access to the database, on the production base, we reduced the execution time to generate more than 500 hr.leaves from several minutes to 21 seconds. A fix to avoid deadlock was already made at https://github.com/odoo/enterprise/pull/10740/files Some contortions were made to avoid changing a signature method in a stable release and thus introducing for each method a second one, with the "batched" implementation, to keep a retro-compatibility for the existing custom code. But surely this could be cleaned in the master version. The old one will be deprecated while waiting to be removed in a few versions. closes odoo/odoo#56534 Taskid: 2256705 X-original-commit: 8c96a887d3f05680c923dcd44f9c70c0e5e0bd32 Related: odoo/enterprise#12670 Signed-off-by: Yannick Tivisse (yti) --- addons/calendar/models/__init__.py | 1 - addons/hr_holidays/models/hr_leave.py | 242 +++++++++-------- .../hr_holidays/models/hr_leave_allocation.py | 13 +- addons/hr_holidays/models/hr_leave_type.py | 28 +- .../hr_holidays/tests/test_company_leave.py | 43 +++ addons/mrp/models/mrp_workcenter.py | 2 +- addons/resource/models/resource.py | 255 ++++++++++++------ addons/resource/models/resource_mixin.py | 82 ++++-- addons/resource/tests/test_resource.py | 108 ++++---- odoo/models.py | 22 +- 10 files changed, 520 insertions(+), 276 deletions(-) diff --git a/addons/calendar/models/__init__.py b/addons/calendar/models/__init__.py index 3f5cb8f1be5..3cacb6bca87 100644 --- a/addons/calendar/models/__init__.py +++ b/addons/calendar/models/__init__.py @@ -9,7 +9,6 @@ from . import calendar_alarm_manager from . import calendar_attendee from . import calendar_contact from . import calendar_event_type -from . import calendar_event from . import calendar_recurrence from . import mail_activity from . import res_users diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index 2525ad737ac..fed3140b8a8 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -119,7 +119,7 @@ class HolidaysRequest(models.Model): ('refuse', 'Refused'), ('validate1', 'Second Approval'), ('validate', 'Approved') - ], string='Status', compute='_compute_state', store=True, tracking=True, copy=False, + ], string='Status', compute='_compute_state', store=True, tracking=True, copy=False, readonly=False, help="The status is set to 'To Submit', when a time off request is created." + "\nThe status is 'To Approve', when time off request is confirmed by user." + "\nThe status is 'Refused', when time off request is refused by manager." + @@ -498,8 +498,9 @@ class HolidaysRequest(models.Model): start_dt = start_dt.replace(tzinfo=UTC) if not end_dt.tzinfo: end_dt = end_dt.replace(tzinfo=UTC) - intervals = calendar._attendance_intervals(start_dt, end_dt, holiday.employee_id.resource_id) \ - - calendar._leave_intervals(start_dt, end_dt, None) # Substract Global Leaves + resource = holiday.employee_id.resource_id + intervals = calendar._attendance_intervals_batch(start_dt, end_dt, resource)[resource.id] \ + - calendar._leave_intervals_batch(start_dt, end_dt, None)[False] # Substract Global Leaves number_of_hours = sum((stop - start).total_seconds() / 3600 for start, stop, dummy in intervals) else: number_of_hours = holiday._get_number_of_days(holiday.date_from, holiday.date_to, holiday.employee_id.id)['hours'] @@ -564,16 +565,19 @@ class HolidaysRequest(models.Model): @api.constrains('state', 'number_of_days', 'holiday_status_id') def _check_holidays(self): + mapped_days = self.mapped('holiday_status_id').get_employees_days(self.mapped('employee_id').ids) for holiday in self: if holiday.holiday_type != 'employee' or not holiday.employee_id or holiday.holiday_status_id.allocation_type == 'no': continue - leave_days = holiday.holiday_status_id.get_days(holiday.employee_id.id)[holiday.holiday_status_id.id] + leave_days = mapped_days[holiday.employee_id.id][holiday.holiday_status_id.id] if float_compare(leave_days['remaining_leaves'], 0, precision_digits=2) == -1 or float_compare(leave_days['virtual_remaining_leaves'], 0, precision_digits=2) == -1: raise ValidationError(_('The number of remaining time off is not sufficient for this time off type.\n' 'Please also check the time off waiting for validation.')) @api.constrains('date_from', 'date_to', 'employee_id') def _check_date_state(self): + if self.env.context.get('leave_skip_state_check'): + return for holiday in self: if holiday.state in ['cancel', 'refuse', 'validate1', 'validate']: raise ValidationError(_("This modification is not allowed in the current state.")) @@ -582,7 +586,7 @@ class HolidaysRequest(models.Model): """ Returns a float equals to the timedelta between two dates given as string.""" if employee_id: employee = self.env['hr.employee'].browse(employee_id) - return employee._get_work_days_data(date_from, date_to) + return employee._get_work_days_data_batch(date_from, date_to)[employee.id] today_hours = self.env.company.resource_calendar_id.get_work_hours_count( datetime.combine(date_from.date(), time.min), @@ -730,43 +734,52 @@ class HolidaysRequest(models.Model): # Is probably handled via ir.rule raise AccessError(_('You don\'t have the rights to apply second approval on a time off request')) - @api.model - def create(self, values): + @api.model_create_multi + def create(self, vals_list): """ Override to avoid automatic logging of creation """ if not self._context.get('leave_fast_create'): - employee_id = values.get('employee_id', False) - leave_type_id = values.get('holiday_status_id') - leave_type = self.env['hr.leave.type'].browse(leave_type_id) - # Handle automatic department_id - if not values.get('department_id'): - values.update({'department_id': self.env['hr.employee'].browse(employee_id).department_id.id}) + leave_types = self.env['hr.leave.type'].browse([values.get('holiday_status_id') for values in vals_list if values.get('holiday_status_id')]) + mapped_validation_type = {leave_type.id: leave_type.leave_validation_type for leave_type in leave_types} - # Handle no_validation - if leave_type.leave_validation_type == 'no_validation': - values.update({'state': 'confirm'}) + for values in vals_list: + employee_id = values.get('employee_id', False) + leave_type_id = values.get('holiday_status_id') + # Handle automatic department_id + if not values.get('department_id'): + values.update({'department_id': self.env['hr.employee'].browse(employee_id).department_id.id}) - # Handle double validation - if leave_type.leave_validation_type == 'both': - self._check_double_validation_rules(employee_id, values.get('state', False)) + # Handle no_validation + if mapped_validation_type[leave_type_id] == 'no_validation': + values.update({'state': 'confirm'}) - holiday = super(HolidaysRequest, self.with_context(mail_create_nosubscribe=True)).create(values) - if not self._context.get('leave_fast_create'): - # Everything that is done here must be done using sudo because we might - # have different create and write rights - # eg : holidays_user can create a leave request with validation_type = 'manager' for someone else - # but they can only write on it if they are leave_manager_id - holiday_sudo = holiday.sudo() - holiday_sudo.add_follower(employee_id) - if holiday.validation_type == 'manager': - holiday_sudo.message_subscribe(partner_ids=holiday.employee_id.leave_manager_id.partner_id.ids) - if holiday.validation_type == 'no_validation': - # Automatic validation should be done in sudo, because user might not have the rights to do it by himself - holiday_sudo.action_validate() - holiday_sudo.message_subscribe(partner_ids=[holiday._get_responsible_for_approval().partner_id.id]) - holiday_sudo.message_post(body=_("The time off has been automatically approved"), subtype_xmlid="mail.mt_comment") # Message from OdooBot (sudo) - elif not self._context.get('import_file'): - holiday_sudo.activity_update() - return holiday + if 'state' not in values: + # To mimic the behavior of compute_state that was always triggered, as the field was readonly + values['state'] = 'confirm' if mapped_validation_type[leave_type_id] != 'no_validation' else 'draft' + + # Handle double validation + if mapped_validation_type[leave_type_id] == 'both': + self._check_double_validation_rules(employee_id, values.get('state', False)) + + holidays = super(HolidaysRequest, self.with_context(mail_create_nosubscribe=True)).create(vals_list) + + for holiday in holidays: + if not self._context.get('leave_fast_create'): + # Everything that is done here must be done using sudo because we might + # have different create and write rights + # eg : holidays_user can create a leave request with validation_type = 'manager' for someone else + # but they can only write on it if they are leave_manager_id + holiday_sudo = holiday.sudo() + holiday_sudo.add_follower(employee_id) + if holiday.validation_type == 'manager': + holiday_sudo.message_subscribe(partner_ids=holiday.employee_id.leave_manager_id.partner_id.ids) + if holiday.validation_type == 'no_validation': + # Automatic validation should be done in sudo, because user might not have the rights to do it by himself + holiday_sudo.action_validate() + holiday_sudo.message_subscribe(partner_ids=[holiday._get_responsible_for_approval().partner_id.id]) + holiday_sudo.message_post(body=_("The time off has been automatically approved"), subtype_xmlid="mail.mt_comment") # Message from OdooBot (sudo) + elif not self._context.get('import_file'): + holiday_sudo.activity_update() + return holidays def write(self, values): is_officer = self.env.user.has_group('hr_holidays.group_hr_holidays_user') or self.env.is_superuser() @@ -832,20 +845,15 @@ class HolidaysRequest(models.Model): """ This method will create entry in resource calendar time off object at the time of holidays validated :returns: created `resource.calendar.leaves` """ - vals_list = [] - for leave in self: - date_from = fields.Datetime.from_string(leave.date_from) - date_to = fields.Datetime.from_string(leave.date_to) - - vals_list.append({ - 'name': leave.name, - 'date_from': fields.Datetime.to_string(date_from), - 'holiday_id': leave.id, - 'date_to': fields.Datetime.to_string(date_to), - 'resource_id': leave.employee_id.resource_id.id, - 'calendar_id': leave.employee_id.resource_calendar_id.id, - 'time_type': leave.holiday_status_id.time_type, - }) + vals_list = [{ + 'name': leave.name, + 'date_from': leave.date_from, + 'holiday_id': leave.id, + 'date_to': leave.date_to, + 'resource_id': leave.employee_id.resource_id.id, + 'calendar_id': leave.employee_id.resource_calendar_id.id, + 'time_type': leave.holiday_status_id.time_type, + } for leave in self] return self.env['resource.calendar.leaves'].sudo().create(vals_list) def _remove_resource_leave(self): @@ -857,39 +865,52 @@ class HolidaysRequest(models.Model): by creating a calendar event and a resource time off. """ holidays = self.filtered(lambda request: request.holiday_type == 'employee') holidays._create_resource_leave() - for holiday in holidays.filtered(lambda l: l.holiday_status_id.create_calendar_meeting): - meeting_values = holiday._prepare_holidays_meeting_values() - meeting = self.env['calendar.event'].with_context(no_mail_to_attendees=True, active_model=self._name).create(meeting_values) - holiday.write({'meeting_id': meeting.id}) + meeting_holidays = holidays.filtered(lambda l: l.holiday_status_id.create_calendar_meeting) + if meeting_holidays: + meeting_values = meeting_holidays._prepare_holidays_meeting_values() + meetings = self.env['calendar.event'].with_context( + no_mail_to_attendees=True, + active_model=self._name + ).create(meeting_values) + for holiday, meeting in zip(meeting_holidays, meetings): + holiday.meeting_id = meeting def _prepare_holidays_meeting_values(self): - self.ensure_one() - calendar = self.employee_id.resource_calendar_id or self.env.company.resource_calendar_id - if self.leave_type_request_unit == 'hour': - meeting_name = _("%s on Time Off : %.2f hour(s)") % (self.employee_id.name or self.category_id.name, self.number_of_hours_display) - else: - meeting_name = _("%s on Time Off : %.2f day(s)") % (self.employee_id.name or self.category_id.name, self.number_of_days) - meeting_values = { - 'name': meeting_name, - 'duration': self.number_of_days * (calendar.hours_per_day or HOURS_PER_DAY), - 'description': self.notes, - 'user_id': self.user_id.id, - 'start': self.date_from, - 'stop': self.date_to, - 'allday': False, - 'privacy': 'confidential', - 'event_tz': self.user_id.tz, - 'activity_ids': [(5, 0, 0)], - } - # Add the partner_id (if exist) as an attendee - if self.user_id and self.user_id.partner_id: - meeting_values['partner_ids'] = [ - (4, self.user_id.partner_id.id)] - return meeting_values + result = [] + company_calendar = self.env.company.resource_calendar_id + for holiday in self: + calendar = holiday.employee_id.resource_calendar_id or company_calendar + if holiday.leave_type_request_unit == 'hour': + meeting_name = _("%s on Time Off : %.2f hour(s)") % (holiday.employee_id.name or holiday.category_id.name, holiday.number_of_hours_display) + else: + meeting_name = _("%s on Time Off : %.2f day(s)") % (holiday.employee_id.name or holiday.category_id.name, holiday.number_of_days) + meeting_values = { + 'name': meeting_name, + 'duration': holiday.number_of_days * (calendar.hours_per_day or HOURS_PER_DAY), + 'description': holiday.notes, + 'user_id': holiday.user_id.id, + 'start': holiday.date_from, + 'stop': holiday.date_to, + 'allday': False, + 'privacy': 'confidential', + 'event_tz': holiday.user_id.tz, + 'activity_ids': [(5, 0, 0)], + } + # Add the partner_id (if exist) as an attendee + if holiday.user_id and holiday.user_id.partner_id: + meeting_values['partner_ids'] = [ + (4, holiday.user_id.partner_id.id)] + result.append(meeting_values) + return result + # YTI TODO: Remove me in master def _prepare_holiday_values(self, employee): + return self._prepare_employees_holiday_values(employee)[0] + + def _prepare_employees_holiday_values(self, employees): self.ensure_one() - values = { + work_days_data = employees._get_work_days_data_batch(self.date_from, self.date_to) + return [{ 'name': self.name, 'holiday_type': 'employee', 'holiday_status_id': self.holiday_status_id.id, @@ -898,11 +919,11 @@ class HolidaysRequest(models.Model): 'request_date_from': self.date_from, 'request_date_to': self.date_to, 'notes': self.notes, - 'number_of_days': employee._get_work_days_data(self.date_from, self.date_to)['days'], + 'number_of_days': work_days_data[employee.id]['days'], 'parent_id': self.id, - 'employee_id': employee.id - } - return values + 'employee_id': employee.id, + 'state': 'validate', + } for employee in employees] def action_draft(self): if any(holiday.state not in ['confirm', 'refuse'] for holiday in self): @@ -988,18 +1009,20 @@ class HolidaysRequest(models.Model): if holiday.leave_type_request_unit != 'day' or any(l.leave_type_request_unit == 'hour' for l in conflicting_leaves): raise ValidationError(_('You can not have 2 time off that overlaps on the same day.')) + # keep track of conflicting leaves states before refusal + target_states = {l.id: l.state for l in conflicting_leaves} + conflicting_leaves.action_refuse() + split_leaves_vals = [] for conflicting_leave in conflicting_leaves: if conflicting_leave.leave_type_request_unit == 'half_day' and conflicting_leave.request_unit_half: - conflicting_leave.action_refuse() continue + # Leaves in days - split_leaves = self.env['hr.leave'] - target_state = conflicting_leave.state - conflicting_leave.action_refuse() if conflicting_leave.date_from < holiday.date_from: before_leave_vals = conflicting_leave.copy_data({ 'date_from': conflicting_leave.date_from.date(), 'date_to': holiday.date_from.date() + timedelta(days=-1), + 'state': target_states[conflicting_leave.id], })[0] before_leave = self.env['hr.leave'].new(before_leave_vals) before_leave._compute_date_from_to() @@ -1015,48 +1038,39 @@ class HolidaysRequest(models.Model): # new instead of create. As the leave is not actually created yet, the sql # constraint didn't check date_from < date_to yet. if before_leave.date_from < before_leave.date_to: - split_leaves |= self.env['hr.leave'].with_context( - tracking_disable=True, - mail_activity_automation_skip=True, - leave_fast_create=True - ).create(before_leave._convert_to_write(before_leave._cache)) + split_leaves_vals.append(before_leave._convert_to_write(before_leave._cache)) if conflicting_leave.date_to > holiday.date_to: after_leave_vals = conflicting_leave.copy_data({ 'date_from': holiday.date_to.date() + timedelta(days=1), 'date_to': conflicting_leave.date_to.date(), + 'state': target_states[conflicting_leave.id], })[0] after_leave = self.env['hr.leave'].new(after_leave_vals) after_leave._compute_date_from_to() # Could happen for part-time contract, that time off is not necessary # anymore. if after_leave.date_from < after_leave.date_to: - split_leaves |= self.env['hr.leave'].with_context( - tracking_disable=True, - mail_activity_automation_skip=True, - leave_fast_create=True - ).create(after_leave._convert_to_write(after_leave._cache)) - for split_leave in split_leaves: - if target_state == 'draft': - continue - if target_state == 'confirm' and split_leave.state == 'draft': - split_leave.action_confirm() - elif target_state == 'validate1': - split_leave.action_confirm() - split_leave.action_approve() - elif target_state == 'validate': - split_leave.action_confirm() - split_leave.action_validate() + split_leaves_vals.append(after_leave._convert_to_write(after_leave._cache)) - values = [holiday._prepare_holiday_values(employee) for employee in employees] + split_leaves = self.env['hr.leave'].with_context( + tracking_disable=True, + mail_activity_automation_skip=True, + leave_fast_create=True, + leave_skip_state_check=True + ).create(split_leaves_vals) + + split_leaves.filtered(lambda l: l.state in 'validate')._validate_leave_request() + + values = holiday._prepare_employees_holiday_values(employees) leaves = self.env['hr.leave'].with_context( tracking_disable=True, mail_activity_automation_skip=True, leave_fast_create=True, + leave_skip_state_check=True, ).create(values) - leaves.action_approve() - # FIXME RLi: This does not make sense, only the parent should be in validation_type both - if leaves and leaves[0].validation_type == 'both': - leaves.action_validate() + + leaves._validate_leave_request() + employee_requests = self.filtered(lambda hol: hol.holiday_type == 'employee') employee_requests._validate_leave_request() if not self.env.context.get('leave_fast_create'): @@ -1230,5 +1244,5 @@ class HolidaysRequest(models.Model): dfrom = datetime.combine(fields.Date.from_string(date_from), time.min).replace(tzinfo=UTC) dto = datetime.combine(fields.Date.from_string(date_to), time.max).replace(tzinfo=UTC) - works = {d[0].date() for d in calendar._work_intervals(dfrom, dto)} + works = {d[0].date() for d in calendar._work_intervals_batch(dfrom, dto)[False]} return {fields.Date.to_string(day.date()): (day.date() not in works) for day in rrule(DAILY, dfrom, until=dto)} diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py index a71917c9044..82266cc7ea5 100644 --- a/addons/hr_holidays/models/hr_leave_allocation.py +++ b/addons/hr_holidays/models/hr_leave_allocation.py @@ -197,8 +197,15 @@ class HolidaysAllocation(models.Model): if period_start <= start_date: period_start = start_date - worked = holiday.employee_id._get_work_days_data(period_start, period_end, domain=[('holiday_id.holiday_status_id.unpaid', '=', True), ('time_type', '=', 'leave')])['days'] - left = holiday.employee_id._get_leave_days_data(period_start, period_end, domain=[('holiday_id.holiday_status_id.unpaid', '=', True), ('time_type', '=', 'leave')])['days'] + employee = holiday.employee_id + worked = employee._get_work_days_data_batch( + period_start, period_end, + domain=[('holiday_id.holiday_status_id.unpaid', '=', True), ('time_type', '=', 'leave')] + )[employee.id]['days'] + left = employee._get_leave_days_data_batch( + period_start, period_end, + domain=[('holiday_id.holiday_status_id.unpaid', '=', True), ('time_type', '=', 'leave')] + )[employee.id]['days'] prorata = worked / (left + worked) if worked else 0 days_to_give = holiday.number_per_interval @@ -206,7 +213,7 @@ class HolidaysAllocation(models.Model): # As we encode everything in days in the database we need to convert # the number of hours into days for this we use the # mean number of hours set on the employee's calendar - days_to_give = days_to_give / (holiday.employee_id.resource_calendar_id.hours_per_day or HOURS_PER_DAY) + days_to_give = days_to_give / (employee.resource_calendar_id.hours_per_day or HOURS_PER_DAY) values['number_of_days'] = holiday.number_of_days + days_to_give * prorata if holiday.accrual_limit > 0: diff --git a/addons/hr_holidays/models/hr_leave_type.py b/addons/hr_holidays/models/hr_leave_type.py index 772cde2ee93..48baf174ab6 100644 --- a/addons/hr_holidays/models/hr_leave_type.py +++ b/addons/hr_holidays/models/hr_leave_type.py @@ -188,24 +188,37 @@ class HolidaysType(models.Model): return [('id', 'in', valid_leave_types.ids)] + # YTI TODO: Remove me in master def get_days(self, employee_id): - # need to use `dict` constructor to create a dict per id - result = dict((id, dict(max_leaves=0, leaves_taken=0, remaining_leaves=0, virtual_remaining_leaves=0, virtual_leaves_taken=0)) for id in self.ids) + return self.get_employees_days([employee_id])[employee_id] + + def get_employees_days(self, employee_ids): + result = { + employee_id: { + leave_type.id: { + 'max_leaves': 0, + 'leaves_taken': 0, + 'remaining_leaves': 0, + 'virtual_remaining_leaves': 0, + 'virtual_leaves_taken': 0, + } for leave_type in self + } for employee_id in employee_ids + } requests = self.env['hr.leave'].search([ - ('employee_id', '=', employee_id), + ('employee_id', 'in', employee_ids), ('state', 'in', ['confirm', 'validate1', 'validate']), ('holiday_status_id', 'in', self.ids) ]) allocations = self.env['hr.leave.allocation'].search([ - ('employee_id', '=', employee_id), + ('employee_id', 'in', employee_ids), ('state', 'in', ['confirm', 'validate1', 'validate']), ('holiday_status_id', 'in', self.ids) ]) for request in requests: - status_dict = result[request.holiday_status_id.id] + status_dict = result[request.employee_id.id][request.holiday_status_id.id] status_dict['virtual_remaining_leaves'] -= (request.number_of_hours_display if request.leave_type_request_unit == 'hour' else request.number_of_days) @@ -221,7 +234,7 @@ class HolidaysType(models.Model): else request.number_of_days) for allocation in allocations.sudo(): - status_dict = result[allocation.holiday_status_id.id] + status_dict = result[allocation.employee_id.id][allocation.holiday_status_id.id] if allocation.state == 'validate': # note: add only validated allocation even for the virtual # count; otherwise pending then refused allocation allow @@ -235,7 +248,6 @@ class HolidaysType(models.Model): status_dict['remaining_leaves'] += (allocation.number_of_hours_display if allocation.type_request_unit == 'hour' else allocation.number_of_days) - return result @api.model @@ -265,7 +277,7 @@ class HolidaysType(models.Model): employee_id = self._get_contextual_employee_id() if employee_id: - data_days = self.get_days(employee_id) + data_days = self.get_employees_days([employee_id])[employee_id] for holiday_status in self: result = data_days.get(holiday_status.id, {}) diff --git a/addons/hr_holidays/tests/test_company_leave.py b/addons/hr_holidays/tests/test_company_leave.py index 70010823aa2..c34c3c93fa2 100644 --- a/addons/hr_holidays/tests/test_company_leave.py +++ b/addons/hr_holidays/tests/test_company_leave.py @@ -6,6 +6,7 @@ from datetime import date, datetime from odoo.tests import tagged from odoo.tests.common import SavepointCase + @tagged('company_leave') class TestCompanyLeave(SavepointCase): """ Test leaves for a whole company, conflict resolutions """ @@ -278,3 +279,45 @@ class TestCompanyLeave(SavepointCase): self.assertEqual(all_leaves[2].date_to, datetime(2020, 1, 10, 16, 0)) self.assertEqual(all_leaves[2].number_of_days, 1) self.assertEqual(all_leaves[2].state, 'validate') + + def test_leave_whole_company_07(self): + # Test Case 7: Try to create a bank holidays for a lot of + # employees, and check the performances + # 100 employees - 15 already on holidays that day + + employees = self.env['hr.employee'].create([{ + 'name': 'Employee %s' % i, + 'company_id': self.company.id + } for i in range(100)]) + + leaves = self.env['hr.leave'].create([{ + 'name': 'Holiday - %s' % employee.name, + 'employee_id': employee.id, + 'holiday_status_id': self.paid_time_off.id, + 'request_date_from': date(2020, 3, 29), + 'request_date_to': date(2020, 4, 1), + 'number_of_days': 3, + } for employee in employees[0:15]]) + leaves._compute_date_from_to() + + company_leave = self.env['hr.leave'].create({ + 'name': 'Bank Holiday', + 'holiday_type': 'company', + 'mode_company_id': self.company.id, + 'holiday_status_id': self.bank_holiday.id, + 'date_from': date(2020, 4, 1), + 'request_date_from': date(2020, 4, 1), + 'date_to': date(2020, 4, 1), + 'request_date_to': date(2020, 4, 1), + 'number_of_days': 1, + }) + company_leave._compute_date_from_to() + + count = 865 + with self.assertQueryCount(__system__=count, admin=count): + # Original query count: 1987 + # Without tracking/activity context keys: 5154 + company_leave.action_validate() + + leaves = self.env['hr.leave'].search([('holiday_status_id', '=', self.bank_holiday.id)]) + self.assertEqual(len(leaves), 102) diff --git a/addons/mrp/models/mrp_workcenter.py b/addons/mrp/models/mrp_workcenter.py index f90d8e96fff..969461a4d61 100644 --- a/addons/mrp/models/mrp_workcenter.py +++ b/addons/mrp/models/mrp_workcenter.py @@ -333,7 +333,7 @@ class MrpWorkcenterProductivity(models.Model): d2 = fields.Datetime.from_string(blocktime.date_end) diff = d2 - d1 if (blocktime.loss_type not in ('productive', 'performance')) and blocktime.workcenter_id.resource_calendar_id: - r = blocktime.workcenter_id._get_work_days_data(d1, d2)['hours'] + r = blocktime.workcenter_id._get_work_days_data_batch(d1, d2)[blocktime.workcenter_id.id]['hours'] blocktime.duration = round(r * 60, 2) else: blocktime.duration = round(diff.total_seconds() / 60.0, 2) diff --git a/addons/resource/models/resource.py b/addons/resource/models/resource.py index 69cc1168067..4f75a2e1edf 100644 --- a/addons/resource/models/resource.py +++ b/addons/resource/models/resource.py @@ -335,15 +335,26 @@ class ResourceCalendar(models.Model): # -------------------------------------------------- # Computation API # -------------------------------------------------- + # YTI TODO: Remove me in master def _attendance_intervals(self, start_dt, end_dt, resource=None, domain=None, tz=None): + if resource is None: + resource = self.env['resource.resource'] + return self._attendance_intervals_batch( + start_dt, end_dt, resources=resource, domain=domain, tz=tz + )[resource.id] + + def _attendance_intervals_batch(self, start_dt, end_dt, resources=None, domain=None, tz=None): """ Return the attendance intervals in the given datetime range. The returned intervals are expressed in specified tz or in the resource's timezone. """ + self.ensure_one() + resources = self.env['resource.resource'] if not resources else resources assert start_dt.tzinfo and end_dt.tzinfo self.ensure_one() combine = datetime.combine - resource_ids = [resource.id, False] if resource else [False] + resources_list = list(resources) + [self.env['resource.resource']] + resource_ids = [r.id for r in resources_list] domain = domain if domain is not None else [] domain = expression.AND([domain, [ ('calendar_id', '=', self.id), @@ -351,50 +362,80 @@ class ResourceCalendar(models.Model): ('display_type', '=', False), ]]) - # express all dates and times in specified tz or in the resource's timezone - tz = tz if tz else timezone((resource or self).tz) - start_dt = start_dt.astimezone(tz) - end_dt = end_dt.astimezone(tz) - # for each attendance spec, generate the intervals in the date range - result = [] + cache_dates = defaultdict(dict) + cache_deltas = defaultdict(dict) + result = defaultdict(list) for attendance in self.env['resource.calendar.attendance'].search(domain): - start = start_dt.date() - if attendance.date_from: - start = max(start, attendance.date_from) - until = end_dt.date() - if attendance.date_to: - until = min(until, attendance.date_to) - if attendance.week_type: - start_week_type = int(math.floor((start.toordinal()-1)/7) % 2) - if start_week_type != int(attendance.week_type): - # start must be the week of the attendance - # if it's not the case, we must remove one week - start = start + relativedelta(weeks=-1) - weekday = int(attendance.dayofweek) + for resource in resources_list: + # express all dates and times in specified tz or in the resource's timezone + tz = tz if tz else timezone((resource or self).tz) + if (tz, start_dt) in cache_dates: + start = cache_dates[(tz, start_dt)] + else: + start = start_dt.astimezone(tz) + cache_dates[(tz, start_dt)] = start + if (tz, end_dt) in cache_dates: + end = cache_dates[(tz, end_dt)] + else: + end = end_dt.astimezone(tz) + cache_dates[(tz, end_dt)] = end - if self.two_weeks_calendar and attendance.week_type: - days = rrule(WEEKLY, start, interval=2, until=until, byweekday=weekday) - else: - days = rrule(DAILY, start, until=until, byweekday=weekday) + start = start.date() + if attendance.date_from: + start = max(start, attendance.date_from) + until = end.date() + if attendance.date_to: + until = min(until, attendance.date_to) + if attendance.week_type: + start_week_type = int(math.floor((start.toordinal()-1)/7) % 2) + if start_week_type != int(attendance.week_type): + # start must be the week of the attendance + # if it's not the case, we must remove one week + start = start + relativedelta(weeks=-1) + weekday = int(attendance.dayofweek) - for day in days: - # attendance hours are interpreted in the resource's timezone - dt0 = tz.localize(combine(day, float_to_time(attendance.hour_from))) - dt1 = tz.localize(combine(day, float_to_time(attendance.hour_to))) - result.append((max(start_dt, dt0), min(end_dt, dt1), attendance)) + if self.two_weeks_calendar and attendance.week_type: + days = rrule(WEEKLY, start, interval=2, until=until, byweekday=weekday) + else: + days = rrule(DAILY, start, until=until, byweekday=weekday) - return Intervals(result) + for day in days: + # attendance hours are interpreted in the resource's timezone + hour_from = attendance.hour_from + if (tz, day, hour_from) in cache_deltas: + dt0 = cache_deltas[(tz, day, hour_from)] + else: + dt0 = tz.localize(combine(day, float_to_time(hour_from))) + cache_deltas[(tz, day, hour_from)] = dt0 + + hour_to = attendance.hour_to + if (tz, day, hour_to) in cache_deltas: + dt1 = cache_deltas[(tz, day, hour_to)] + else: + dt1 = tz.localize(combine(day, float_to_time(hour_to))) + cache_deltas[(tz, day, hour_to)] = dt1 + result[resource.id].append((max(start_dt, dt0), min(end_dt, dt1), attendance)) + return {r.id: Intervals(result[r.id]) for r in resources_list} def _leave_intervals(self, start_dt, end_dt, resource=None, domain=None, tz=None): + if resource is None: + resource = self.env['resource.resource'] + return self._leave_intervals_batch( + start_dt, end_dt, resources=resource, domain=domain, tz=tz + )[resource.id] + + def _leave_intervals_batch(self, start_dt, end_dt, resources=None, domain=None, tz=None): """ Return the leave intervals in the given datetime range. The returned intervals are expressed in specified tz or in the calendar's timezone. """ + resources = self.env['resource.resource'] if not resources else resources assert start_dt.tzinfo and end_dt.tzinfo self.ensure_one() # for the computation, express all datetimes in UTC - resource_ids = [resource.id, False] if resource else [False] + resources_list = list(resources) + [self.env['resource.resource']] + resource_ids = [r.id for r in resources_list] if domain is None: domain = [('time_type', '=', 'leave')] domain = domain + [ @@ -405,33 +446,78 @@ class ResourceCalendar(models.Model): ] # retrieve leave intervals in (start_dt, end_dt) - tz = tz if tz else timezone((resource or self).tz) - start_dt = start_dt.astimezone(tz) - end_dt = end_dt.astimezone(tz) - result = [] + result = defaultdict(lambda: []) + tz_dates = {} for leave in self.env['resource.calendar.leaves'].search(domain): - dt0 = string_to_datetime(leave.date_from).astimezone(tz) - dt1 = string_to_datetime(leave.date_to).astimezone(tz) - result.append((max(start_dt, dt0), min(end_dt, dt1), leave)) + for resource in resources_list: + if leave.resource_id.id not in [False, resource.id]: + continue + tz = tz if tz else timezone((resource or self).tz) + if (tz, start_dt) in tz_dates: + start = tz_dates[(tz, start_dt)] + else: + start = start_dt.astimezone(tz) + tz_dates[(tz, start_dt)] = start + if (tz, end_dt) in tz_dates: + end = tz_dates[(tz, end_dt)] + else: + end = end_dt.astimezone(tz) + tz_dates[(tz, end_dt)] = end + dt0 = string_to_datetime(leave.date_from).astimezone(tz) + dt1 = string_to_datetime(leave.date_to).astimezone(tz) + result[resource.id].append((max(start, dt0), min(end, dt1), leave)) - return Intervals(result) + return {r.id: Intervals(result[r.id]) for r in resources_list} + # YTI TODO: Remove me in master def _work_intervals(self, start_dt, end_dt, resource=None, domain=None, tz=None): + if resource is None: + resource = self.env['resource.resource'] + return self._work_intervals_batch( + start_dt, end_dt, resources=resource, domain=domain, tz=tz + )[resource.id] + + def _work_intervals_batch(self, start_dt, end_dt, resources=None, domain=None, tz=None): """ Return the effective work intervals between the given datetimes. """ - return (self._attendance_intervals(start_dt, end_dt, resource, tz=tz) - - self._leave_intervals(start_dt, end_dt, resource, domain, tz=tz)) + if not resources: + resources = self.env['resource.resource'] + resources_list = [resources] + else: + resources_list = list(resources) + + attendance_intervals = self._attendance_intervals_batch(start_dt, end_dt, resources, tz=tz) + leave_intervals = self._leave_intervals_batch(start_dt, end_dt, resources, domain, tz=tz) + return { + r.id: (attendance_intervals[r.id] - leave_intervals[r.id]) for r in resources_list + } def _unavailable_intervals(self, start_dt, end_dt, resource=None, domain=None, tz=None): + if resource is None: + resource = self.env['resource.resource'] + return self._unavailable_intervals_batch( + start_dt, end_dt, resources=resource, domain=domain, tz=tz + )[resource.id] + + def _unavailable_intervals_batch(self, start_dt, end_dt, resources=None, domain=None, tz=None): """ Return the unavailable intervals between the given datetimes. """ - work_intervals = self._work_intervals(start_dt, end_dt, resource, domain, tz) - work_intervals = [(start, stop) for start, stop, meta in work_intervals] - # start + flatten(intervals) + end - work_intervals = [start_dt] + list(chain.from_iterable(work_intervals)) + [end_dt] - # put it back to UTC - work_intervals = list(map(lambda dt: dt.astimezone(utc), work_intervals)) - # pick groups of two - work_intervals = list(zip(work_intervals[0::2], work_intervals[1::2])) - return work_intervals + if not resources: + resources = self.env['resource.resource'] + resources_list = [resources] + else: + resources_list = list(resources) + + resources_work_intervals = self._work_intervals_batch(start_dt, end_dt, resources, domain, tz) + result = {} + for resource in resources_list: + work_intervals = [(start, stop) for start, stop, meta in resources_work_intervals[resource.id]] + # start + flatten(intervals) + end + work_intervals = [start_dt] + list(chain.from_iterable(work_intervals)) + [end_dt] + # put it back to UTC + work_intervals = list(map(lambda dt: dt.astimezone(utc), work_intervals)) + # pick groups of two + work_intervals = list(zip(work_intervals[0::2], work_intervals[1::2])) + result[resource.id] = work_intervals + return result # -------------------------------------------------- # Private Methods / Helpers @@ -449,7 +535,7 @@ class ResourceCalendar(models.Model): # compute number of days as quarters days = sum( - float_utils.round(ROUNDING_FACTOR * day_hours[day] / day_total[day]) / ROUNDING_FACTOR + float_utils.round(ROUNDING_FACTOR * day_hours[day] / day_total[day]) / ROUNDING_FACTOR if day_total[day] else 0 for day in day_hours ) return { @@ -457,20 +543,31 @@ class ResourceCalendar(models.Model): 'hours': sum(day_hours.values()), } + # YTI TODO: Remove me in master def _get_day_total(self, from_datetime, to_datetime, resource=None): + if resource is None: + resource = self.env['resource.resource'] + return self._get_resources_day_total(from_datetime, to_datetime, resources=resource)[resource.id] + + def _get_resources_day_total(self, from_datetime, to_datetime, resources=None): """ @return dict with hours of attendance in each day between `from_datetime` and `to_datetime` """ self.ensure_one() + resources = self.env['resource.resource'] if not resources else resources + resources_list = list(resources) + [self.env['resource.resource']] # total hours per day: retrieve attendances with one extra day margin, # in order to compute the total hours on the first and last days from_full = from_datetime - timedelta(days=1) to_full = to_datetime + timedelta(days=1) - intervals = self._attendance_intervals(from_full, to_full, resource=resource) - day_total = defaultdict(float) - for start, stop, meta in intervals: - day_total[start.date()] += (stop - start).total_seconds() / 3600 - return day_total + intervals = self._attendance_intervals_batch(from_full, to_full, resources=resources) + + result = defaultdict(lambda: defaultdict(float)) + for resource in resources_list: + day_total = result[resource.id] + for start, stop, meta in intervals[resource.id]: + day_total[start.date()] += (stop - start).total_seconds() / 3600 + return result def _get_closest_work_time(self, dt, match_end=False, resource=None, search_range=None): """Return the closest work interval boundary within the search range. @@ -484,6 +581,9 @@ class ResourceCalendar(models.Model): def interval_dt(interval): return interval[1 if match_end else 0] + if resource is None: + resource = self.env['resource.resource'] + if not dt.tzinfo or search_range and not (search_range[0].tzinfo and search_range[1].tzinfo): raise ValueError('Provided datetimes needs to be timezoned') dt = dt.astimezone(timezone(self.tz)) @@ -497,7 +597,7 @@ class ResourceCalendar(models.Model): if not range_start <= dt <= range_end: return None work_intervals = sorted( - self._work_intervals(range_start, range_end, resource), + self._work_intervals_batch(range_start, range_end, resource)[resource.id], key=lambda i: abs(interval_dt(i) - dt), ) return interval_dt(work_intervals[0]) if work_intervals else None @@ -524,9 +624,9 @@ class ResourceCalendar(models.Model): end_dt = end_dt.replace(tzinfo=utc) if compute_leaves: - intervals = self._work_intervals(start_dt, end_dt, domain=domain) + intervals = self._work_intervals_batch(start_dt, end_dt, domain=domain)[False] else: - intervals = self._attendance_intervals(start_dt, end_dt) + intervals = self._attendance_intervals_batch(start_dt, end_dt)[False] return sum( (stop - start).total_seconds() / 3600 @@ -549,13 +649,13 @@ class ResourceCalendar(models.Model): from_datetime, dummy = make_aware(from_datetime) to_datetime, dummy = make_aware(to_datetime) - day_total = self._get_day_total(from_datetime, to_datetime) + day_total = self._get_resources_day_total(from_datetime, to_datetime)[False] # actual hours per day if compute_leaves: - intervals = self._work_intervals(from_datetime, to_datetime, domain=domain) + intervals = self._work_intervals_batch(from_datetime, to_datetime, domain=domain)[False] else: - intervals = self._attendance_intervals(from_datetime, to_datetime) + intervals = self._attendance_intervals_batch(from_datetime, to_datetime)[False] return self._get_days_data(intervals, day_total) @@ -735,17 +835,18 @@ class ResourceResource(models.Model): if record.time_efficiency == 0: raise ValidationError(_('The efficiency factor cannot be equal to 0.')) - @api.model - def create(self, values): - if values.get('company_id') and not values.get('calendar_id'): - values['calendar_id'] = self.env['res.company'].browse(values['company_id']).resource_calendar_id.id - if not values.get('tz'): - # retrieve timezone on user or calendar - tz = (self.env['res.users'].browse(values.get('user_id')).tz or - self.env['resource.calendar'].browse(values.get('calendar_id')).tz) - if tz: - values['tz'] = tz - return super(ResourceResource, self).create(values) + @api.model_create_multi + def create(self, vals_list): + for values in vals_list: + if values.get('company_id') and not values.get('calendar_id'): + values['calendar_id'] = self.env['res.company'].browse(values['company_id']).resource_calendar_id.id + if not values.get('tz'): + # retrieve timezone on user or calendar + tz = (self.env['res.users'].browse(values.get('user_id')).tz or + self.env['resource.calendar'].browse(values.get('calendar_id')).tz) + if tz: + values['tz'] = tz + return super(ResourceResource, self).create(vals_list) @api.returns('self', lambda value: value.id) def copy(self, default=None): @@ -811,9 +912,13 @@ class ResourceResource(models.Model): start_datetime = timezone_datetime(start) end_datetime = timezone_datetime(end) resource_mapping = {} + calendar_mapping = defaultdict(lambda: self.env['resource.resource']) for resource in self: - calendar = resource.calendar_id - resource_mapping[resource.id] = calendar._unavailable_intervals(start_datetime, end_datetime, resource) + calendar_mapping[resource.calendar_id] |= resource + + for calendar, resources in calendar_mapping.items(): + resources_unavailable_intervals = calendar._unavailable_intervals_batch(start_datetime, end_datetime, resources) + resource_mapping.update(resources_unavailable_intervals) return resource_mapping diff --git a/addons/resource/models/resource_mixin.py b/addons/resource/models/resource_mixin.py index 368528bf61a..f3404d5ad2c 100644 --- a/addons/resource/models/resource_mixin.py +++ b/addons/resource/models/resource_mixin.py @@ -54,7 +54,18 @@ class ResourceMixin(models.AbstractModel): default['resource_calendar_id'] = resource.calendar_id.id return super(ResourceMixin, self).copy_data(default) + # YTI TODO: Remove me in master def _get_work_days_data(self, from_datetime, to_datetime, compute_leaves=True, calendar=None, domain=None): + self.ensure_one() + return self._get_work_days_data_batch( + from_datetime, + to_datetime, + compute_leaves=compute_leaves, + calendar=calendar, + domain=domain + )[self.id] + + def _get_work_days_data_batch(self, from_datetime, to_datetime, compute_leaves=True, calendar=None, domain=None): """ By default the resource calendar is used, but it can be changed using the `calendar` argument. @@ -65,24 +76,44 @@ class ResourceMixin(models.AbstractModel): Returns a dict {'days': n, 'hours': h} containing the quantity of working time expressed as days and as hours. """ - resource = self.resource_id - calendar = calendar or self.resource_calendar_id + resources = self.mapped('resource_id') + mapped_employees = {e.resource_id.id: e.id for e in self} + result = {} # naive datetimes are made explicit in UTC from_datetime = timezone_datetime(from_datetime) to_datetime = timezone_datetime(to_datetime) - day_total = calendar._get_day_total(from_datetime, to_datetime, resource) + mapped_resources = defaultdict(lambda: self.env['resource.resource']) + for record in self: + mapped_resources[calendar or record.resource_calendar_id] |= record.resource_id - # actual hours per day - if compute_leaves: - intervals = calendar._work_intervals(from_datetime, to_datetime, resource, domain) - else: - intervals = calendar._attendance_intervals(from_datetime, to_datetime, resource) + for calendar, calendar_resources in mapped_resources.items(): + day_total = calendar._get_resources_day_total(from_datetime, to_datetime, calendar_resources) - return calendar._get_days_data(intervals, day_total) + # actual hours per day + if compute_leaves: + intervals = calendar._work_intervals_batch(from_datetime, to_datetime, calendar_resources, domain) + else: + intervals = calendar._attendance_intervals_batch(from_datetime, to_datetime, calendar_resources) + for calendar_resource in calendar_resources: + result[calendar_resource.id] = calendar._get_days_data(intervals[calendar_resource.id], day_total[calendar_resource.id]) + + # convert "resource: result" into "employee: result" + return {mapped_employees[r.id]: result[r.id] for r in resources} + + # YTI TODO: Remove me in master def _get_leave_days_data(self, from_datetime, to_datetime, calendar=None, domain=None): + self.ensure_one() + return self._get_leave_days_data_batch( + from_datetime, + to_datetime, + calendar=calendar, + domain=domain + )[self.id] + + def _get_leave_days_data_batch(self, from_datetime, to_datetime, calendar=None, domain=None): """ By default the resource calendar is used, but it can be changed using the `calendar` argument. @@ -93,20 +124,33 @@ class ResourceMixin(models.AbstractModel): Returns a dict {'days': n, 'hours': h} containing the number of leaves expressed as days and as hours. """ - resource = self.resource_id - calendar = calendar or self.resource_calendar_id + resources = self.mapped('resource_id') + mapped_employees = {e.resource_id.id: e.id for e in self} + result = {} # naive datetimes are made explicit in UTC from_datetime = timezone_datetime(from_datetime) to_datetime = timezone_datetime(to_datetime) - day_total = calendar._get_day_total(from_datetime, to_datetime, resource) + mapped_resources = defaultdict(lambda: self.env['resource.resource']) + for record in self: + mapped_resources[calendar or record.resource_calendar_id] |= record.resource_id - # compute actual hours per day - attendances = calendar._attendance_intervals(from_datetime, to_datetime, resource) - leaves = calendar._leave_intervals(from_datetime, to_datetime, resource, domain) + for calendar, calendar_resources in mapped_resources.items(): + day_total = calendar._get_resources_day_total(from_datetime, to_datetime, calendar_resources) - return calendar._get_days_data(attendances & leaves, day_total) + # compute actual hours per day + attendances = calendar._attendance_intervals_batch(from_datetime, to_datetime, calendar_resources) + leaves = calendar._leave_intervals_batch(from_datetime, to_datetime, calendar_resources, domain) + + for calendar_resource in calendar_resources: + result[calendar_resource.id] = calendar._get_days_data( + attendances[calendar_resource.id] & leaves[calendar_resource.id], + day_total[calendar_resource.id] + ) + + # convert "resource: result" into "employee: result" + return {mapped_employees[r.id]: result[r.id] for r in resources} def _adjust_to_calendar(self, start, end): resource_results = self.resource_id._adjust_to_calendar(start, end) @@ -136,7 +180,7 @@ class ResourceMixin(models.AbstractModel): if not to_datetime.tzinfo: to_datetime = to_datetime.replace(tzinfo=utc) - intervals = calendar._work_intervals(from_datetime, to_datetime, resource, domain) + intervals = calendar._work_intervals_batch(from_datetime, to_datetime, resource, domain)[resource.id] result = defaultdict(float) for start, stop, meta in intervals: result[start.date()] += (stop - start).total_seconds() / 3600 @@ -162,8 +206,8 @@ class ResourceMixin(models.AbstractModel): if not to_datetime.tzinfo: to_datetime = to_datetime.replace(tzinfo=utc) - attendances = calendar._attendance_intervals(from_datetime, to_datetime, resource) - leaves = calendar._leave_intervals(from_datetime, to_datetime, resource, domain) + attendances = calendar._attendance_intervals_batch(from_datetime, to_datetime, resource)[resource.id] + leaves = calendar._leave_intervals_batch(from_datetime, to_datetime, resource, domain)[resource.id] result = [] for start, stop, leave in (leaves & attendances): hours = (stop - start).total_seconds() / 3600 diff --git a/addons/resource/tests/test_resource.py b/addons/resource/tests/test_resource.py index 280447adf57..02e0997d6ad 100644 --- a/addons/resource/tests/test_resource.py +++ b/addons/resource/tests/test_resource.py @@ -510,26 +510,26 @@ class TestResMixin(TestResourceCommon): # Looking at Jean's calendar # Viewing it as Jean - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 6, 16, 0, 0, tzinfo=self.jean.tz), - ) + )[self.jean.id] self.assertEqual(data, {'days': 5, 'hours': 40}) # Viewing it as Patel # Views from 2018/04/01 20:00:00 to 2018/04/06 12:00:00 - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.patel.tz), datetime_tz(2018, 4, 6, 16, 0, 0, tzinfo=self.patel.tz), - ) + )[self.jean.id] self.assertEqual(data, {'days': 4.5, 'hours': 36}) # We see only 36 hours # Viewing it as John # Views from 2018/04/02 09:00:00 to 2018/04/07 02:00:00 - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.john.tz), datetime_tz(2018, 4, 6, 16, 0, 0, tzinfo=self.john.tz), - ) + )[self.jean.id] # still showing as 5 days because of rounding, but we see only 39 hours self.assertEqual(data, {'days': 4.875, 'hours': 39}) @@ -537,33 +537,33 @@ class TestResMixin(TestResourceCommon): # Viewing it as Jean # Views from 2018/04/01 15:00:00 to 2018/04/06 14:00:00 - data = self.john._get_work_days_data( + data = self.john._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.jean.tz), - ) + )[self.john.id] self.assertEqual(data, {'days': 1.4375, 'hours': 13}) # Viewing it as Patel # Views from 2018/04/01 11:00:00 to 2018/04/06 10:00:00 - data = self.john._get_work_days_data( + data = self.john._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.patel.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.patel.tz), - ) + )[self.john.id] self.assertEqual(data, {'days': 1.1875, 'hours': 10}) # Viewing it as John - data = self.john._get_work_days_data( + data = self.john._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.john.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.john.tz), - ) + )[self.john.id] self.assertEqual(data, {'days': 2, 'hours': 20}) # using Jean as a timezone reference - data = self.john._get_work_days_data( + data = self.john._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.john.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.john.tz), calendar=self.calendar_jean, - ) + )[self.john.id] self.assertEqual(data, {'days': 5, 'hours': 40}) # half days @@ -575,18 +575,18 @@ class TestResMixin(TestResourceCommon): 'date_to': datetime_str(2018, 4, 2, 14, 0, 0, tzinfo=self.jean.tz), }) - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.jean.tz), - ) + )[self.jean.id] self.assertEqual(data, {'days': 4.5, 'hours': 36}) # using John as a timezone reference, leaves are outside attendances - data = self.john._get_work_days_data( + data = self.john._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.john.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.john.tz), calendar=self.calendar_jean, - ) + )[self.john.id] self.assertEqual(data, {'days': 5, 'hours': 40}) leave.unlink() @@ -600,10 +600,10 @@ class TestResMixin(TestResourceCommon): 'date_to': datetime_str(2018, 4, 2, 10, 0, 0, tzinfo=self.jean.tz), }) - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.jean.tz), - ) + )[self.jean.id] self.assertEqual(data, {'days': 5, 'hours': 40}) leave.unlink() @@ -617,10 +617,10 @@ class TestResMixin(TestResourceCommon): 'date_to': datetime_str(2018, 4, 2, 10, 0, 1, tzinfo=self.jean.tz), }) - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.jean.tz), - ) + )[self.jean.id] self.assertEqual(data['days'], 5) self.assertAlmostEqual(data['hours'], 40, 2) @@ -644,28 +644,28 @@ class TestResMixin(TestResourceCommon): }) # Jean asks to see how much leave he has taken - data = self.jean._get_leave_days_data( + data = self.jean._get_leave_days_data_batch( datetime_tz(2018, 4, 9, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 13, 23, 59, 59, tzinfo=self.jean.tz), - ) + )[self.jean.id] # Sees only 1 day and 8 hours because, as john is in UTC-7 the second leave is not in # the attendances of Jean self.assertEqual(data, {'days': 1, 'hours': 8}) # Patel Asks to see when Jean has taken some leaves # Patel should see the same - data = self.jean._get_leave_days_data( + data = self.jean._get_leave_days_data_batch( datetime_tz(2018, 4, 9, 0, 0, 0, tzinfo=self.patel.tz), datetime_tz(2018, 4, 13, 23, 59, 59, tzinfo=self.patel.tz), - ) + )[self.jean.id] self.assertEqual(data, {'days': 1, 'hours': 8}) # use Patel as a resource, jean's leaves are not visible - datas = self.patel._get_leave_days_data( + datas = self.patel._get_leave_days_data_batch( datetime_tz(2018, 4, 9, 0, 0, 0, tzinfo=self.patel.tz), datetime_tz(2018, 4, 13, 23, 59, 59, tzinfo=self.patel.tz), calendar=self.calendar_jean, - ) + )[self.patel.id] self.assertEqual(datas['days'], 0) self.assertEqual(datas['hours'], 0) @@ -691,10 +691,10 @@ class TestResMixin(TestResourceCommon): # John asks how much leaves he has # He sees that he has only 15 hours of leave in his attendances - data = self.john._get_leave_days_data( + data = self.john._get_leave_days_data_batch( datetime_tz(2018, 4, 9, 0, 0, 0, tzinfo=self.john.tz), datetime_tz(2018, 4, 13, 23, 59, 59, tzinfo=self.john.tz), - ) + )[self.john.id] self.assertEqual(data, {'days': 0.9375, 'hours': 10}) # half days @@ -706,10 +706,10 @@ class TestResMixin(TestResourceCommon): 'date_to': datetime_str(2018, 4, 2, 14, 0, 0, tzinfo=self.jean.tz), }) - data = self.jean._get_leave_days_data( + data = self.jean._get_leave_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.jean.tz), - ) + )[self.jean.id] self.assertEqual(data, {'days': 0.5, 'hours': 4}) leave.unlink() @@ -723,10 +723,10 @@ class TestResMixin(TestResourceCommon): 'date_to': datetime_str(2018, 4, 2, 10, 0, 0, tzinfo=self.jean.tz), }) - data = self.jean._get_leave_days_data( + data = self.jean._get_leave_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.jean.tz), - ) + )[self.jean.id] self.assertEqual(data, {'days': 0, 'hours': 0}) leave.unlink() @@ -740,10 +740,10 @@ class TestResMixin(TestResourceCommon): 'date_to': datetime_str(2018, 4, 2, 10, 0, 1, tzinfo=self.jean.tz), }) - data = self.jean._get_leave_days_data( + data = self.jean._get_leave_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jean.tz), datetime_tz(2018, 4, 6, 23, 0, 0, tzinfo=self.jean.tz), - ) + )[self.jean.id] self.assertEqual(data['days'], 0) self.assertAlmostEqual(data['hours'], 0, 2) @@ -964,50 +964,50 @@ class TestTimezones(TestResourceCommon): def test_work_data(self): # 09-04-2018 10:00:00 - 13-04-2018 18:00:00 - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 9, 8, 0, 0), datetime_tz(2018, 4, 13, 16, 0, 0), - ) + )[self.jean.id] self.assertEqual(data, {'days': 4.75, 'hours': 38}) # 09-04-2018 00:00:00 - 13-04-2018 08:00:00 - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 9, 8, 0, 0, tzinfo=self.tz3), datetime_tz(2018, 4, 13, 16, 0, 0, tzinfo=self.tz3), - ) + )[self.jean.id] self.assertEqual(data, {'days': 4, 'hours': 32}) # 09-04-2018 08:00:00 - 14-04-2018 12:00:00 - data = self.jean._get_work_days_data( + data = self.jean._get_work_days_data_batch( datetime_tz(2018, 4, 9, 8, 0, 0, tzinfo=self.tz2), datetime_tz(2018, 4, 13, 16, 0, 0, tzinfo=self.tz4), - ) + )[self.jean.id] self.assertEqual(data, {'days': 5, 'hours': 40}) # Jules with 2 weeks calendar # 02-04-2018 00:00:00 - 6-04-2018 23:59:59 - data = self.jules._get_work_days_data( + data = self.jules._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jules.tz), datetime_tz(2018, 4, 6, 23, 59, 59, tzinfo=self.jules.tz), - ) + )[self.jules.id] self.assertEqual(data, {'days': 4, 'hours': 30}) # Jules with 2 weeks calendar # 02-04-2018 00:00:00 - 14-04-2018 23:59:59 - data = self.jules._get_work_days_data( + data = self.jules._get_work_days_data_batch( datetime_tz(2018, 4, 2, 0, 0, 0, tzinfo=self.jules.tz), datetime_tz(2018, 4, 14, 23, 59, 59, tzinfo=self.jules.tz), - ) + )[self.jules.id] self.assertEqual(data, {'days': 6, 'hours': 46}) # Jules with 2 weeks calendar # 12-29-2014 00:00:00 - 27-12-2019 23:59:59 => 261 weeks # 130 weeks type 1: 131*4 = 524 days and 131*30 = 3930 hours # 131 weeks type 2: 130*2 = 260 days and 130*16 = 2080 hours - data = self.jules._get_work_days_data( + data = self.jules._get_work_days_data_batch( datetime_tz(2014, 12, 29, 0, 0, 0, tzinfo=self.jules.tz), datetime_tz(2019, 12, 27, 23, 59, 59, tzinfo=self.jules.tz), - ) + )[self.jules.id] self.assertEqual(data, {'days': 784, 'hours': 6010}) def test_leave_data(self): @@ -1020,24 +1020,24 @@ class TestTimezones(TestResourceCommon): }) # 09-04-2018 10:00:00 - 13-04-2018 18:00:00 - data = self.jean._get_leave_days_data( + data = self.jean._get_leave_days_data_batch( datetime_tz(2018, 4, 9, 8, 0, 0), datetime_tz(2018, 4, 13, 16, 0, 0), - ) + )[self.jean.id] self.assertEqual(data, {'days': 0.5, 'hours': 4}) # 09-04-2018 00:00:00 - 13-04-2018 08:00:00 - data = self.jean._get_leave_days_data( + data = self.jean._get_leave_days_data_batch( datetime_tz(2018, 4, 9, 8, 0, 0, tzinfo=self.tz3), datetime_tz(2018, 4, 13, 16, 0, 0, tzinfo=self.tz3), - ) + )[self.jean.id] self.assertEqual(data, {'days': 0.75, 'hours': 6}) # 09-04-2018 08:00:00 - 14-04-2018 12:00:00 - data = self.jean._get_leave_days_data( + data = self.jean._get_leave_days_data_batch( datetime_tz(2018, 4, 9, 8, 0, 0, tzinfo=self.tz2), datetime_tz(2018, 4, 13, 16, 0, 0, tzinfo=self.tz4), - ) + )[self.jean.id] self.assertEqual(data, {'days': 0.75, 'hours': 6}) def test_leaves(self): diff --git a/odoo/models.py b/odoo/models.py index 991573a4155..329e64bca7c 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3869,7 +3869,27 @@ Fields: else: other_fields.add(field) - # insert a row with the given columns + # Insert rows one by one + # - as records don't all specify the same columns, code building batch-insert query + # was very complex + # - and the gains were low, so not worth spending so much complexity + # + # It also seems that we have to be careful with INSERTs in batch, because they have the + # same problem as SELECTs: + # If we inject a lot of data in a single query, we fall into pathological perfs in + # terms of SQL parser and the execution of the query itself. + # In SELECT queries, we inject max 1000 ids (integers) when we can, because we know + # that this limit is well managed by PostgreSQL. + # In INSERT queries, we inject integers (small) and larger data (TEXT blocks for + # example). + # + # The problem then becomes: how to "estimate" the right size of the batch to have + # good performance? + # + # This requires extensive testing, and it was prefered not to introduce INSERTs in + # batch, to avoid regressions as much as possible. + # + # That said, we haven't closed the door completely. query = "INSERT INTO {} ({}) VALUES ({}) RETURNING id".format( quote(self._table), ", ".join(quote(name) for name, fmt, val in columns),