From f58368210c9c962c84cdcd2e2a799c5fc6183a13 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 28 Aug 2019 13:02:23 +0000 Subject: [PATCH] [ADD] base_import: batching (& some other features) various UI changes ------------------ * renamed "test import" button to "test" * move relation fields thing to debug mode * remove "Defer parent/child computation" option as it was deprecated / removed from the backend in 80f1ac3599ee630e7556ff4fcf3eb192931eccd8, turns out this checkbox existed inactive for longer than it's been of any use (added in 68cb2ade09206b0abb3f7c8dccb5d35a6499f802 on 2017-11-29, made non-operating on 2018-01-24, that's so sad) batching -------- * add support for batching imports (skip & limit parameters) * modify client to use batched imports & properly adapt responses so it still looks like a single import for the client (more or less) e.g. update row numbers in error messages, etc... * properly handle partial imports though * disable usual loading throbber to have a single progress notification displayed continuously throughout all the batches: the normal throbber only shows after 3s of waiting for an RPC response, so it would keep flashing in and out (appear 3s into a batch's import then disappear at the end only to reappear 3s into the next batch's loading) NOTE: the limit is row-wise. If a record straddles the limit (because of nested O2M records), the record is imported in full and the "next row" is whatever row follows the record. This means a limit of 10 can lead to an import of 17 lines, and as the progress indicator is in records# the increments can jump around. Task 2059448 --- addons/base_import/models/base_import.py | 32 ++- addons/base_import/models/test_models.py | 41 ++-- .../static/src/js/import_action.js | 198 +++++++++++++++--- .../static/src/scss/base_import.scss | 65 +++--- .../static/src/xml/base_import.xml | 72 ++++--- addons/base_import/tests/test_base_import.py | 182 ++++++++++++++-- odoo/addons/test_impex/tests/test_load.py | 10 +- odoo/models.py | 26 ++- 8 files changed, 487 insertions(+), 139 deletions(-) diff --git a/addons/base_import/models/base_import.py b/addons/base_import/models/base_import.py index d3c932e0358..a7cc6a2ae4b 100644 --- a/addons/base_import/models/base_import.py +++ b/addons/base_import/models/base_import.py @@ -600,6 +600,17 @@ class Import(models.TransientModel): has_relational_match = any(len(match) > 1 for field, match in matches.items() if match) advanced_mode = has_relational_header or has_relational_match + batch = False + batch_cutoff = options.get('limit') + if batch_cutoff: + if count > batch_cutoff: + batch = len(preview) > batch_cutoff + else: + batch = bool(next( + itertools.islice(rows, batch_cutoff - count, None), + None + )) + return { 'fields': fields, 'matches': matches or False, @@ -609,6 +620,7 @@ class Import(models.TransientModel): 'options': options, 'advanced_mode': advanced_mode, 'debug': self.user_has_groups('base.group_no_one'), + 'batch': batch, } except Exception as error: # Due to lazy generators, UnicodeDecodeError (for @@ -662,7 +674,9 @@ class Import(models.TransientModel): if any(row) ] - return data, import_fields + # slicing needs to happen after filtering out empty rows as the + # data offsets from load are post-filtering + return data[options.get('skip'):], import_fields @api.model def _remove_currency_symbol(self, value): @@ -882,7 +896,8 @@ class Import(models.TransientModel): _logger.info('importing %d rows...', len(data)) name_create_enabled_fields = options.pop('name_create_enabled_fields', {}) - model = self.env[self.res_model].with_context(import_file=True, name_create_enabled_fields=name_create_enabled_fields) + import_limit = options.pop('limit', None) + model = self.env[self.res_model].with_context(import_file=True, name_create_enabled_fields=name_create_enabled_fields, _import_limit=import_limit) import_result = model.load(import_fields, data) _logger.info('done') @@ -920,10 +935,21 @@ class Import(models.TransientModel): }) if 'name' in import_fields: index_of_name = import_fields.index('name') - import_result['name'] = [x[index_of_name] for x in data] + skipped = options.get('skip', 0) + # pad front as data doesn't contain anythig for skipped lines + r = import_result['name'] = [''] * skipped + # only add names for the window being imported + r.extend(x[index_of_name] for x in data[:import_limit]) + # pad back (though that's probably not useful) + r.extend([''] * (len(data) - (import_limit or 0))) else: import_result['name'] = [] + skip = options.get('skip', 0) + # convert load's internal nextrow to the imported file's + if import_result['nextrow']: # don't update if nextrow = 0 (= no nextrow) + import_result['nextrow'] += skip + return import_result _SEPARATORS = [' ', '/', '-', ''] diff --git a/addons/base_import/models/test_models.py b/addons/base_import/models/test_models.py index 24415d464c1..af0255739c3 100644 --- a/addons/base_import/models/test_models.py +++ b/addons/base_import/models/test_models.py @@ -2,85 +2,86 @@ from odoo import fields, models -def name(suffix_name): +def model(suffix_name): return 'base_import.tests.models.%s' % suffix_name class Char(models.Model): - _name = name('char') + _name = model('char') _description = 'Tests : Base Import Model, Character' value = fields.Char() class CharRequired(models.Model): - _name = name('char.required') + _name = model('char.required') _description = 'Tests : Base Import Model, Character required' value = fields.Char(required=True) class CharReadonly(models.Model): - _name = name('char.readonly') + _name = model('char.readonly') _description = 'Tests : Base Import Model, Character readonly' value = fields.Char(readonly=True) class CharStates(models.Model): - _name = name('char.states') + _name = model('char.states') _description = 'Tests : Base Import Model, Character states' value = fields.Char(readonly=True, states={'draft': [('readonly', False)]}) class CharNoreadonly(models.Model): - _name = name('char.noreadonly') + _name = model('char.noreadonly') _description = 'Tests : Base Import Model, Character No readonly' value = fields.Char(readonly=True, states={'draft': [('invisible', True)]}) class CharStillreadonly(models.Model): - _name = name('char.stillreadonly') + _name = model('char.stillreadonly') _description = 'Tests : Base Import Model, Character still readonly' value = fields.Char(readonly=True, states={'draft': [('readonly', True)]}) # TODO: complex field (m2m, o2m, m2o) class M2o(models.Model): - _name = name('m2o') + _name = model('m2o') _description = 'Tests : Base Import Model, Many to One' - value = fields.Many2one(name('m2o.related')) + value = fields.Many2one(model('m2o.related')) class M2oRelated(models.Model): - _name = name('m2o.related') + _name = model('m2o.related') _description = 'Tests : Base Import Model, Many to One related' value = fields.Integer(default=42) class M2oRequired(models.Model): - _name = name('m2o.required') + _name = model('m2o.required') _description = 'Tests : Base Import Model, Many to One required' - value = fields.Many2one(name('m2o.required.related'), required=True) + value = fields.Many2one(model('m2o.required.related'), required=True) class M2oRequiredRelated(models.Model): - _name = name('m2o.required.related') + _name = model('m2o.required.related') _description = 'Tests : Base Import Model, Many to One required related' value = fields.Integer(default=42) class O2m(models.Model): - _name = name('o2m') + _name = model('o2m') _description = 'Tests : Base Import Model, One to Many' - value = fields.One2many(name('o2m.child'), 'parent_id') + name = fields.Char() + value = fields.One2many(model('o2m.child'), 'parent_id') class O2mChild(models.Model): - _name = name('o2m.child') + _name = model('o2m.child') _description = 'Tests : Base Import Model, One to Many child' - parent_id = fields.Many2one(name('o2m')) + parent_id = fields.Many2one(model('o2m')) value = fields.Integer() class PreviewModel(models.Model): - _name = name('preview') + _name = model('preview') _description = 'Tests : Base Import Model Preview' name = fields.Char('Name') @@ -88,7 +89,7 @@ class PreviewModel(models.Model): othervalue = fields.Integer(string='Other Variable') class FloatModel(models.Model): - _name = name('float') + _name = model('float') _description = 'Tests: Base Import Model Float' value = fields.Float() @@ -96,7 +97,7 @@ class FloatModel(models.Model): currency_id = fields.Many2one('res.currency') class ComplexModel(models.Model): - _name = name('complex') + _name = model('complex') _description = 'Tests: Base Import Model Complex' f = fields.Float() diff --git a/addons/base_import/static/src/js/import_action.js b/addons/base_import/static/src/js/import_action.js index c75d4dbda7f..57974f33a0a 100644 --- a/addons/base_import/static/src/js/import_action.js +++ b/addons/base_import/static/src/js/import_action.js @@ -6,6 +6,8 @@ var config = require('web.config'); var core = require('web.core'); var session = require('web.session'); var time = require('web.time'); +var AbstractWebClient = require('web.AbstractWebClient'); +var Loading = require('web.Loading'); var QWeb = core.qweb; var _t = core._t; @@ -194,7 +196,7 @@ var DataImport = AbstractAction.extend({ this.$buttons = $(QWeb.render("ImportView.buttons", this)); this.$buttons.filter('.o_import_validate').on('click', this.validate.bind(this)); this.$buttons.filter('.o_import_import').on('click', this.import.bind(this)); - this.$buttons.filter('.o_import_file_reload').on('click', this.loaded_file.bind(this)); + this.$buttons.filter('.o_import_file_reload').on('click', this.loaded_file.bind(this, null)); this.$buttons.filter('.oe_import_file').on('click', function () { self.$('.o_content .oe_import_file').click(); }); @@ -291,6 +293,9 @@ var DataImport = AbstractAction.extend({ advanced: this.$('input.oe_import_advanced_mode').prop('checked'), keep_matches: this.do_not_change_match, name_create_enabled_fields: {}, + // start at row 1 = skip 0 lines + skip: Number(this.$('#oe_import_row_start').val()) - 1 || 0, + limit: Number(this.$('#oe_import_batch_limit').val()) || null, }; _(this.opts).each(function (opt) { options[opt.name] = @@ -319,7 +324,12 @@ var DataImport = AbstractAction.extend({ }, //- File & settings change section - onfile_loaded: function () { + onfile_loaded: function (event, from, to, arg) { + // arg is null if reload -> don't reset partial import + if (arg != null ) { + this.toggle_partial(null); + } + this.$buttons.filter('.o_import_import, .o_import_validate, .o_import_file_reload').addClass('d-none'); if (!this.$('input.oe_import_file').val()) { return this['settings_changed'](); } this.$('.oe_import_date_format').select2('val', ''); @@ -378,12 +388,16 @@ var DataImport = AbstractAction.extend({ this.$('input.oe_import_advanced_mode').prop('checked', result.advanced_mode); this.$('.oe_import_grid').html(QWeb.render('ImportView.preview', result)); + this.$('.o_import_batch_alert').toggleClass('d-none', !result.batch); + + var messages = []; if (result.headers.length === 1) { + messages.push({type: 'warning', message: _t("A single column was found in the file, this often means the file separator is incorrect")}); + } + + if (!_.isEmpty(messages)) { this.$('.oe_import_options').show(); - this.onresults(null, null, null, {'messages': [{ - type: 'warning', - message: _t("A single column was found in the file, this often means the file separator is incorrect") - }]}); + this.onresults(null, null, null, {'messages': messages}); } // merge option values back in case they were updated/guessed @@ -392,10 +406,8 @@ var DataImport = AbstractAction.extend({ }); this.$('.oe_import_date_format').select2('val', time.strftime_to_moment_format(result.options.date_format)); this.$('.oe_import_datetime_format').val(time.strftime_to_moment_format(result.options.datetime_format)); - if (result.debug === false){ - this.$('.oe_import_tracking').hide(); - this.$('.oe_import_deferparentstore').hide(); - } + // hide all "true debug" options when not in debug mode + this.$('.oe_import_debug_option').toggleClass('d-none', !result.debug); var $fields = this.$('.oe_import_fields input'); this.render_fields_matches(result, $fields); @@ -561,19 +573,28 @@ var DataImport = AbstractAction.extend({ }).get(); var tracking_disable = 'tracking_disable' in kwargs ? kwargs.tracking_disable : !this.$('#oe_import_tracking').prop('checked') - var defer_parent_store = 'defer_parent_store' in kwargs ? kwargs.defer_parent_store : !!this.$('#oe_import_deferparentstore').prop('checked') delete kwargs.tracking_disable; - delete kwargs.defer_parent_store; kwargs.context = _.extend( {}, this.parent_context, - {tracking_disable: tracking_disable, defer_parent_store_computation: defer_parent_store} + {tracking_disable: tracking_disable} ); - return this._rpc({ - model: 'base_import.import', - method: 'do', - args: [this.id, fields, columns, this.import_options()], - kwargs : kwargs, - }).then(null, function (reason) { + var self = this; + this.trigger_up('with_client', {callback: function () { + this.loading.ignore_events = true; + }}); + $.blockUI({message: QWeb.render('Throbber')}); + $(document.body).addClass('o_ui_blocked'); + var opts = this.import_options(); + + var $el = $('.oe_throbber_message'); + var msg = kwargs.dryrun ? _t("%d records tested...") + : _t("%d records successfully imported..."); + opts.callback = function (count) { + $el.text(_.str.sprintf(msg, count)); + }; + + return this._batchedImport(opts, [this.id, fields, columns], kwargs, {done: 0, prev: 0}) + .then(null, function (reason) { var error = reason.message; var event = reason.event; // In case of unexpected exception, convert @@ -582,8 +603,9 @@ var DataImport = AbstractAction.extend({ if (event) { event.preventDefault(); } var msg; - if (error.data.type === 'xhrerror') { - var xhr = error.data.objects[0]; + var errordata = error.data || {}; + if (errordata.type === 'xhrerror') { + var xhr = errordata.objects[0]; switch (xhr.status) { case 504: // gateway timeout msg = _t("Import timed out. Please retry. If you still encounter this issue, the file may be too big for the system's configuration, try to split it (import less records per file)."); @@ -592,7 +614,7 @@ var DataImport = AbstractAction.extend({ msg = _t("An unknown issue occurred during import (possibly lost connection, data limit exceeded or memory limits exceeded). Please retry in case the issue is transient. If the issue still occurs, try to split the file rather than import it at once."); } } else { - msg = (error.data.arguments && error.data.arguments[1] || error.data.arguments[0]) + msg = errordata.arguments && (errordata.arguments[1] || errordata.arguments[0]) || error.message; } @@ -601,7 +623,76 @@ var DataImport = AbstractAction.extend({ record: false, message: msg, }]}); - }) ; + }).finally(function () { + $(document.body).removeClass('o_ui_blocked'); + $.unblockUI(); + self.trigger_up('with_client', {callback: function () { + delete this.loading.ignore_events; + }}); + }); + }, /** + * + * @param opts import options + * @param args positional arguments to pass along (augmented with the options) + * @param kwargs keyword arguments to pass along (directly) + * @param {Object} rec recursion information record + * @param {Number} rec.done how many records have been loaded so far + * @param {Number} rec.prev nextrow of the previous call so we can know + * how many rows the call we're here performing + * will have consumed, and thus by how much we + * need to offset the messages of the *next* call + * @returns {Promise<{name, ids, messages}>} + * @private + */ + _batchedImport: function (opts, args, kwargs, rec) { + opts.callback && opts.callback(rec.done || 0); + var self = this; + return this._rpc({ + model: 'base_import.import', + method: 'do', + args: args.concat([opts]), + kwargs: kwargs + }).then(function (results) { + _.each(results.messages, offset_by(opts.skip)); + if (!kwargs.dryrun && !results.ids) { + // update skip to failed batch + self.$('#oe_import_row_start').val(opts.skip + 1); + if (opts.skip) { + // there's been an error during a "proper" import, stop & warn + // about partial import maybe + results.messages.push({ + type: 'info', + priority: true, + message: _.str.sprintf(_t("This file has been successfully imported up to line %d."), opts.skip) + }); + } + return results; + } + if (!results.nextrow) { + // we're done + return results; + } + + // do the next batch + return self._batchedImport( + // avoid modifying opts in-place + _.defaults({skip: results.nextrow}, opts), + args, kwargs, { + done: rec.done + (results.ids || []).length, + prev: results.nextrow + } + ).then(function (r2) { + return { + name: _.zip(results.name, r2.name).map(function (names) { + return names[0] || names[1]; + }), + ids: (results.ids || []).concat(r2.ids || []), + messages: results.messages.concat(r2.messages), + skip: r2.skip || results.nextrow, + nextrow: r2.nextrow + } + }); + }); }, onvalidate: function () { var prom = this.call_import({ dryrun: true, tracking_disable: true }); @@ -641,19 +732,31 @@ var DataImport = AbstractAction.extend({ type: 'info', message: _t("Everything seems valid.") }); + } else if (event === 'import_failed' && results.ids) { + // both ids in a failed import -> partial import + this.toggle_partial(results); } + // row indexes come back 0-indexed, spreadsheets // display 1-indexed. var offset = 1; // offset more if header if (this.import_options().headers) { offset += 1; } - var messagesSorted = _.sortBy(_(message).groupBy('message'), function (messageGroupped) { - var order = 0; - if (messageGroupped[0].type === 'warning') { - order = fields.length + 1; + var messagesSorted = _.sortBy(_(message).groupBy('message'), function (group) { + if (group[0].priority){ + return -2; } - return order + _.indexOf(fields, messageGroupped[0].field); + + // sort by gravity, then, order of field in list + var order = 0; + switch (group[0].type) { + case 'error': order = 0; break; + case 'warning': order = fields.length + 1; break; + case 'info': order = 2 * (fields.length + 1); break; + default: order = 3 * (fields.length + 1); break; + } + return order + _.indexOf(fields, group[0].field); }); this.$form.addClass('oe_import_error'); @@ -722,6 +825,24 @@ var DataImport = AbstractAction.extend({ }, })); }, + toggle_partial: function (result) { + var $form = this.$('.oe_import'); + var $partial_warning = this.$('.o_import_partial_alert'); + var $partial_count = this.$('.o_import_partial_count'); + if (result == null) { + $partial_warning.addClass('d-none'); + $form.add(this.$buttons).removeClass('o_import_partial_mode'); + var $skip = this.$('#oe_import_row_start'); + $skip.val($skip.attr('value')); + $partial_count.text(''); + return; + } + + this.$('.o_import_batch_alert').addClass('d-none'); + $partial_warning.removeClass('d-none'); + $form.add(this.$buttons).addClass('o_import_partial_mode'); + $partial_count.text((result.skip || 0) + 1); + } }); core.action_registry.add('import', DataImport); @@ -746,6 +867,27 @@ StateMachine.create({ ], }); +Loading.include({ + on_rpc_event: function () { + if (this.ignore_events) { + return + } + this._super.apply(this, arguments); + } +}); +AbstractWebClient.prototype.custom_events['with_client'] = function (ev) { + ev.data.callback.call(this); +}; + +function offset_by(by) { + return function offset_message(msg) { + if (msg.rows) { + msg.rows.from += by; + msg.rows.to += by; + } + } +} + return { DataImport: DataImport, }; diff --git a/addons/base_import/static/src/scss/base_import.scss b/addons/base_import/static/src/scss/base_import.scss index 5499c5c4553..bed08873898 100644 --- a/addons/base_import/static/src/scss/base_import.scss +++ b/addons/base_import/static/src/scss/base_import.scss @@ -14,17 +14,13 @@ text-align: justify } h2 { - margin-top: 0; + margin-top: 0.5em; font-size: large; // override h2 font-size which is too large } .oe_padding { padding: 13px 0; } - .oe_import_advanced_mode { - margin-left: 20px; - } - .oe_import_box { padding: 8px; background: #F0EEEE; @@ -68,6 +64,19 @@ .oe_import_with_file label { font-weight: normal; } + .oe_import_debug_options { + max-width: 800px; + columns: 1; + @include media-breakpoint-up(md) { + columns: 2; + } + // try to keep the batch fields together, doesn't work on firefox & + // not sure how to do that (except by adding intermediate dom + // elements) + .oe_import_batch_limit { + break-before: column; + } + } &.oe_import_preview .oe_import_grid { display: table; @@ -81,27 +90,27 @@ } /* ------------- ERRORS AND WARNINGS REPORT ------------ */ - .oe_import_error_report { - > ul { - padding: 0; + .oe_import_error_report > ul { + padding: 0; + } + .oe_import_report { + list-style: none; + } + .alert { + padding: 0.50rem 1.25rem; + margin: 0.25rem 0; + + a { + @extend .alert-link; + &:hover {opacity: 0.8;} } - .oe_import_report { - padding: 4px; - margin: 2px 0; - list-style: none; - border-radius: $border-radius; - &.bg-error { - @extend .bg-danger; - } - &.text-error { - @extend .text-danger; - } + + // alias -error to -danger + &.alert-error { + @extend .alert-danger; } - .oe_import_report_count, .oe_import_see_all { - color: white; - &:hover { - color: lightgrey; - } + &.text-error { + @extend .text-danger; } } @@ -123,9 +132,13 @@ .select2-default{ color: #F00 !important; } - } - +/* ------------- PARTIAL MODE buttons ------------ */ +// hide import in partial mode, resume otherwise +.o_import_import_full.o_import_partial_mode, +.o_import_import_partial:not(.o_import_partial_mode) { + display: none; +} /* Field dropdown */ .oe_import_selector { diff --git a/addons/base_import/static/src/xml/base_import.xml b/addons/base_import/static/src/xml/base_import.xml index 6d69d6995aa..aa0e956f9fd 100644 --- a/addons/base_import/static/src/xml/base_import.xml +++ b/addons/base_import/static/src/xml/base_import.xml @@ -4,7 +4,7 @@
-
+
@@ -36,34 +36,45 @@
+
+ Due to its large size, the file will be imported by batches. +
+
+ Click 'Resume' to proceed with the import, resuming at line + 0.
+ You can test or reload your file before resuming the import. +
-
+

Map your columns to import

-
- - +
+
+ + +
+
+ + +
+
+ + +
+
+ + +
+
+ + +
-
- - -
- - - -

If the file contains the column names, Odoo can try auto-detecting the field corresponding to the column. This makes imports @@ -95,8 +106,9 @@ - - + + + @@ -132,7 +144,7 @@ -

+

Import preview failed due to: .

For CSV files, you may need to select the correct separator.

Here is the start of the file we could not import:

@@ -141,7 +153,7 @@
  • + t-attf-class="oe_import_report alert alert-#{error_value[0].type}"> diff --git a/addons/base_import/tests/test_base_import.py b/addons/base_import/tests/test_base_import.py index 3f5830a8db2..3fe12d4b7bb 100644 --- a/addons/base_import/tests/test_base_import.py +++ b/addons/base_import/tests/test_base_import.py @@ -1,7 +1,9 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. import base64 +import difflib import io +import pprint import unittest from odoo.tests.common import TransactionCase, can_import @@ -35,8 +37,12 @@ def sorted_fields(fields): class BaseImportCase(TransactionCase): def assertEqualFields(self, fields1, fields2): - self.assertEqual(sorted_fields(fields1), sorted_fields(fields2)) - + f1 = sorted_fields(fields1) + f2 = sorted_fields(fields2) + assert f1 == f2, '\n'.join(difflib.unified_diff( + pprint.pformat(f1).splitlines(), + pprint.pformat(f2).splitlines() + )) class TestBasicFields(BaseImportCase): @@ -94,15 +100,34 @@ class TestO2M(BaseImportCase): return self.env['base_import.import'].get_fields('base_import.tests.models.' + field) def test_shallow(self): - self.assertEqualFields(self.get_fields('o2m'), make_field(field_type='one2many', fields=[ - ID_FIELD, - # FIXME: should reverse field be ignored? - {'id': 'parent_id', 'name': 'parent_id', 'string': 'Parent', 'type': 'many2one', 'required': False, 'fields': [ - {'id': 'parent_id', 'name': 'id', 'string': 'External ID', 'required': False, 'fields': [], 'type': 'id'}, - {'id': 'parent_id', 'name': '.id', 'string': 'Database ID', 'required': False, 'fields': [], 'type': 'id'}, - ]}, - {'id': 'value', 'name': 'value', 'string': 'Value', 'required': False, 'fields': [], 'type': 'integer'}, - ])) + self.assertEqualFields( + self.get_fields('o2m'), [ + ID_FIELD, + {'id': 'name', 'name': 'name', 'string': "Name", 'required': False, 'fields': [], 'type': 'char',}, + { + 'id': 'value', 'name': 'value', 'string': 'Value', + 'required': False, 'type': 'one2many', + 'fields': [ + ID_FIELD, + { + 'id': 'parent_id', 'name': 'parent_id', + 'string': 'Parent', 'type': 'many2one', + 'required': False, 'fields': [ + {'id': 'parent_id', 'name': 'id', + 'string': 'External ID', 'required': False, + 'fields': [], 'type': 'id'}, + {'id': 'parent_id', 'name': '.id', + 'string': 'Database ID', 'required': False, + 'fields': [], 'type': 'id'}, + ] + }, + {'id': 'value', 'name': 'value', 'string': 'Value', + 'required': False, 'fields': [], 'type': 'integer' + }, + ] + } + ] + ) class TestMatchHeadersSingle(TransactionCase): @@ -280,8 +305,6 @@ class TestPreview(TransactionCase): ['bar', '3', '4'], ['qux', '5', '6'], ]) - # Ensure we only have the response fields we expect - self.assertItemsEqual(list(result), ['matches', 'headers', 'fields', 'preview', 'headers_type', 'options', 'advanced_mode', 'debug']) @unittest.skipUnless(can_import('xlrd'), "XLRD module not available") def test_xls_success(self): @@ -310,8 +333,6 @@ class TestPreview(TransactionCase): ['bar', '3', '4'], ['qux', '5', '6'], ]) - # Ensure we only have the response fields we expect - self.assertItemsEqual(list(result), ['matches', 'headers', 'fields', 'preview', 'headers_type', 'options', 'advanced_mode', 'debug']) @unittest.skipUnless(can_import('xlrd.xlsx'), "XLRD/XLSX not available") def test_xlsx_success(self): @@ -340,8 +361,6 @@ class TestPreview(TransactionCase): ['bar', '3', '4'], ['qux', '5', '6'], ]) - # Ensure we only have the response fields we expect - self.assertItemsEqual(list(result), ['matches', 'headers', 'fields', 'preview', 'headers_type', 'options', 'advanced_mode', 'debug']) @unittest.skipUnless(can_import('odf'), "ODFPY not available") def test_ods_success(self): @@ -370,9 +389,6 @@ class TestPreview(TransactionCase): ['bar', '3', '4'], ['aux', '5', '6'], ]) - # Ensure we only have the response fields we expect - self.assertItemsEqual(list(result), ['matches', 'headers', 'fields', 'preview', 'headers_type', 'options', 'advanced_mode', 'debug']) - class test_convert_import_data(TransactionCase): """ Tests conversion of base_import.import input into data which @@ -569,6 +585,132 @@ class test_convert_import_data(TransactionCase): self.assertItemsEqual(data, [data_row]) +class TestBatching(TransactionCase): + def _makefile(self, rows): + f = io.BytesIO() + writer = pycompat.csv_writer(f, quoting=1) + writer.writerow(['name', 'counter']) + for i in range(rows): + writer.writerow(['n_%d' % i, str(i)]) + return f.getvalue() + + def test_recognize_batched(self): + import_wizard = self.env['base_import.import'].create({ + 'res_model': 'base_import.tests.models.preview', + 'file_type': 'text/csv', + }) + + import_wizard.file = self._makefile(10) + result = import_wizard.parse_preview({ + 'quoting': '"', + 'separator': ',', + 'headers': True, + 'limit': 100, + }) + self.assertIsNone(result.get('error')) + self.assertIs(result['batch'], False) + + result = import_wizard.parse_preview({ + 'quoting': '"', + 'separator': ',', + 'headers': True, + 'limit': 5, + }) + self.assertIsNone(result.get('error')) + self.assertIs(result['batch'], True) + + def test_limit_on_lines(self): + """ The limit option should be a limit on the number of *lines* + imported at at time, not the number of *records*. This is relevant + when it comes to embedded o2m. + + A big question is whether we want to round up or down (if the limit + brings us inside a record). Rounding up (aka finishing up the record + we're currently parsing) seems like a better idea: + + * if the first record has so many sub-lines it hits the limit we still + want to import it (it's probably extremely rare but it can happen) + * if we have one line per record, we probably want to import + records not , but if we stop in the middle of the "current + record" we'd always ignore the last record (I think) + """ + f = io.BytesIO() + writer = pycompat.csv_writer(f, quoting=1) + writer.writerow(['name', 'value/value']) + for record in range(10): + writer.writerow(['record_%d' % record, '0']) + for row in range(1, 10): + writer.writerow(['', str(row)]) + + import_wizard = self.env['base_import.import'].create({ + 'res_model': 'base_import.tests.models.o2m', + 'file_type': 'text/csv', + 'file_name': 'things.csv', + 'file': f.getvalue(), + }) + opts = {'quoting': '"', 'separator': ',', 'headers': True} + preview = import_wizard.parse_preview({**opts, 'limit': 15}) + self.assertIs(preview['batch'], True) + + results = import_wizard.do( + ['name', 'value/value'], [], + {**opts, 'limit': 5} + ) + self.assertFalse(results['messages']) + self.assertEqual(len(results['ids']), 1, "should have imported the first record in full, got %s" % results['ids']) + self.assertEqual(results['nextrow'], 10) + + results = import_wizard.do( + ['name', 'value/value'], [], + {**opts, 'limit': 15} + ) + self.assertFalse(results['messages']) + self.assertEqual(len(results['ids']), 2, "should have importe the first two records, got %s" % results['ids']) + self.assertEqual(results['nextrow'], 20) + + + def test_batches(self): + partners_before = self.env['res.partner'].search([]) + opts = {'headers': True, 'separator': ',', 'quoting': '"'} + + import_wizard = self.env['base_import.import'].create({ + 'res_model': 'res.partner', + 'file_type': 'text/csv', + 'file_name': 'clients.csv', + 'file': b"""name,email +a,a@example.com +b,b@example.com +, +c,c@example.com +d,d@example.com +e,e@example.com +f,f@example.com +g,g@example.com +""" + }) + + results = import_wizard.do(['name', 'email'], [], {**opts, 'limit': 1}) + self.assertFalse(results['messages']) + self.assertEqual(len(results['ids']), 1) + # titlerow is ignored by lastrow's counter + self.assertEqual(results['nextrow'], 1) + partners_1 = self.env['res.partner'].search([]) - partners_before + self.assertEqual(partners_1.name, 'a') + + results = import_wizard.do(['name', 'email'], [], {**opts, 'limit': 2, 'skip': 1}) + self.assertFalse(results['messages']) + self.assertEqual(len(results['ids']), 2) + # empty row should also be ignored + self.assertEqual(results['nextrow'], 3) + partners_2 = self.env['res.partner'].search([]) - (partners_before | partners_1) + self.assertEqual(partners_2.mapped('name'), ['b', 'c']) + + results = import_wizard.do(['name', 'email'], [], {**opts, 'limit': 10, 'skip': 3}) + self.assertFalse(results['messages']) + self.assertEqual(len(results['ids']), 4) + self.assertEqual(results['nextrow'], 0) + partners_3 = self.env['res.partner'].search([]) - (partners_before | partners_1 | partners_2) + self.assertEqual(partners_3.mapped('name'), ['d', 'e', 'f', 'g']) class test_failures(TransactionCase): def test_big_attachments(self): diff --git a/odoo/addons/test_impex/tests/test_load.py b/odoo/addons/test_impex/tests/test_load.py index 886b702baab..6010962dca0 100644 --- a/odoo/addons/test_impex/tests/test_load.py +++ b/odoo/addons/test_impex/tests/test_load.py @@ -134,7 +134,7 @@ class test_boolean_field(ImporterCase): def test_empty(self): self.assertEqual( self.import_(['value'], []), - {'ids': [], 'messages': []}) + {'ids': [], 'messages': [], 'nextrow': False}) def test_exported(self): result = self.import_(['value'], [['False'], ['True'], ]) @@ -190,7 +190,7 @@ class test_integer_field(ImporterCase): def test_none(self): self.assertEqual( self.import_(['value'], []), - {'ids': [], 'messages': []}) + {'ids': [], 'messages': [], 'nextrow': False}) def test_empty(self): result = self.import_(['value'], [['']]) @@ -277,7 +277,7 @@ class test_float_field(ImporterCase): def test_none(self): self.assertEqual( self.import_(['value'], []), - {'ids': [], 'messages': []}) + {'ids': [], 'messages': [], 'nextrow': False}) def test_empty(self): result = self.import_(['value'], [['']]) @@ -1030,7 +1030,7 @@ class test_date(ImporterCase): def test_empty(self): self.assertEqual( self.import_(['value'], []), - {'ids': [], 'messages': []}) + {'ids': [], 'messages': [], 'nextrow': False}) def test_basic(self): result = self.import_(['value'], [['2012-02-03']]) @@ -1052,7 +1052,7 @@ class test_datetime(ImporterCase): def test_empty(self): self.assertEqual( self.import_(['value'], []), - {'ids': [], 'messages': []}) + {'ids': [], 'messages': [], 'nextrow': False}) def test_basic(self): result = self.import_(['value'], [['2012-02-03 11:11:11']]) diff --git a/odoo/models.py b/odoo/models.py index 8e8bc8e4db9..439041dd03e 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -858,13 +858,12 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})): :type fields: list(str) :param data: row-major matrix of data to import :type data: list(list(str)) - :returns: {ids: list(int)|False, messages: [Message]} + :returns: {ids: list(int)|False, messages: [Message][, lastrow: int]} """ # determine values of mode, current_module and noupdate mode = self._context.get('mode', 'init') current_module = self._context.get('module', '__import__') noupdate = self._context.get('noupdate', False) - # add current module in context for the conversion of xml ids self = self.with_context(_import_current_module=current_module) @@ -944,9 +943,16 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})): # make 'flush' available to the methods below, in the case where XMLID # resolution fails, for instance flush_self = self.with_context(import_flush=flush) - extracted = flush_self._extract_records(fields, data, log=messages.append) + + # TODO: break load's API instead of smuggling via context? + limit = self._context.get('_import_limit') + if limit is None: + limit = float('inf') + extracted = flush_self._extract_records(fields, data, log=messages.append, limit=limit) + converted = flush_self._convert_records(extracted, log=messages.append) + info = {'rows': {'to': -1}} for id, xid, record, info in converted: if xid: xid = xid if '.' in xid else "%s.%s" % (current_module, xid) @@ -962,7 +968,14 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})): # cancel all changes done to the registry/ormcache self.pool.reset_changes() - return {'ids': ids, 'messages': messages} + nextrow = info['rows']['to'] + 1 + if nextrow < limit: + nextrow = 0 + return { + 'ids': ids, + 'messages': messages, + 'nextrow': nextrow, + } def _add_fake_fields(self, fields): from odoo.fields import Char, Integer @@ -971,8 +984,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})): fields['.id'] = Integer('Database ID') return fields - @api.model - def _extract_records(self, fields_, data, log=lambda a: None): + def _extract_records(self, fields_, data, log=lambda a: None, limit=float('inf')): """ Generates record dicts from the data sequence. The result is a generator of dicts mapping field names to raw @@ -1008,7 +1020,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})): return any(get_o2m_values(row)) and not any(get_nono2m_values(row)) index = 0 - while index < len(data): + while index < len(data) and index < limit: row = data[index] # copy non-relational fields to record dict