From fcbe0df011246a9f9bb060edaeb3b26e564cce7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Mon, 11 Apr 2022 23:24:21 +0200 Subject: [PATCH] Add validate_fields to ModelStorage issue11303 review386191002 --- CHANGELOG | 1 + doc/ref/models.rst | 7 +++ trytond/ir/action.py | 97 +++++++++++++++++------------- trytond/ir/email_.py | 110 +++++++++++++++++++--------------- trytond/ir/lang.py | 20 ++++--- trytond/ir/model.py | 10 ++-- trytond/ir/rule.py | 10 ++-- trytond/ir/sequence.py | 12 +++- trytond/ir/trigger.py | 10 ++-- trytond/model/dictschema.py | 17 ++++-- trytond/model/modelstorage.py | 13 +++- trytond/model/tree.py | 13 ++-- 12 files changed, 196 insertions(+), 124 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 81e68499df..b0dd61a3f1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* Add validate_fields to ModelStorage * Drop support for PyPy and psycopg2cffi * Use a set for field's depends * Relax the constraint on a field's depends diff --git a/doc/ref/models.rst b/doc/ref/models.rst index 89cc1748c0..3dceadfcb3 100644 --- a/doc/ref/models.rst +++ b/doc/ref/models.rst @@ -444,6 +444,13 @@ Class methods: This method must be overridden to add validation and must raise an :exc:`~trytond.model.exceptions.ValidationError` if validation fails. + +.. classmethod:: ModelStorage.validate_fields(records, field_names) + + Validate the integrity of records after modification of the fields. This + method must be overridden to add validation for the field names set and must + raise an exception if validation fails. + Dual methods: .. classmethod:: ModelStorage.save(records) diff --git a/trytond/ir/action.py b/trytond/ir/action.py index ba9fa643e6..a0e22cff4f 100644 --- a/trytond/ir/action.py +++ b/trytond/ir/action.py @@ -170,26 +170,30 @@ class ActionKeyword(ModelSQL, ModelView): table.index_action(['keyword', 'model'], 'add') @classmethod - def validate(cls, actions): - super(ActionKeyword, cls).validate(actions) - for action in actions: - action.check_wizard_model() + def validate_fields(cls, actions, field_names): + super().validate_fields(actions, field_names) + cls.check_wizard_model(actions, field_names) - def check_wizard_model(self): - ActionWizard = Pool().get('ir.action.wizard') - if self.action.type == 'ir.action.wizard': - action_wizards = ActionWizard.search([ - ('action', '=', self.action.id), - ], limit=1) - # could be empty when copying an action - if action_wizards: - action_wizard, = action_wizards - if action_wizard.model: - if not str(self.model).startswith( - '%s,' % action_wizard.model): - raise WizardModelError( - gettext('ir.msg_action_wrong_wizard_model', - name=action_wizard.rec_name)) + @classmethod + def check_wizard_model(cls, actions, field_names=None): + pool = Pool() + ActionWizard = pool.get('ir.action.wizard') + if field_names and not (field_names & {'action', 'model'}): + return + for action in actions: + if action.action.type == 'ir.action.wizard': + action_wizards = ActionWizard.search([ + ('action', '=', action.action.id), + ], limit=1) + # could be empty when copying an action + if action_wizards: + action_wizard, = action_wizards + if action_wizard.model: + if not str(action.model).startswith( + '%s,' % action_wizard.model): + raise WizardModelError( + gettext('ir.msg_action_wrong_wizard_model', + name=action_wizard.rec_name)) @staticmethod def _convert_vals(vals): @@ -701,21 +705,24 @@ class ActionReport(ActionMixin, ModelSQL, ModelView): self._template_cache.set(self.id, template) @classmethod - def validate(cls, reports): - super().validate(reports) - for report in reports: - report.check_record_name() + def validate_fields(cls, reports, field_names): + super().validate_fields(reports, field_names) + cls.check_record_name(reports, field_names) - def check_record_name(self): - if not self.record_name: + @classmethod + def check_record_name(cls, reports, field_names=None): + if field_names and 'record_name' not in field_names: return - try: - TextTemplate(self.record_name) - except Exception as exception: - raise ValidationError( - gettext('ir.msg_report_invalid_record_name', - report=self.rec_name, - exception=exception)) from exception + for report in reports: + if not report.record_name: + return + try: + TextTemplate(report.record_name) + except Exception as exception: + raise ValidationError(gettext( + 'ir.msg_report_invalid_record_name', + report=report.rec_name, + exception=exception)) from exception class ActionActWindow(ActionMixin, ModelSQL, ModelView): @@ -790,8 +797,12 @@ class ActionActWindow(ActionMixin, ModelSQL, ModelView): def validate(cls, actions): super(ActionActWindow, cls).validate(actions) cls.check_views(actions) - cls.check_domain(actions) - cls.check_context(actions) + + @classmethod + def validate_fields(cls, actions, field_names): + super().validate_fields(actions, field_names) + cls.check_domain(actions, field_names) + cls.check_context(actions, field_names) @classmethod def check_views(cls, actions): @@ -825,8 +836,10 @@ class ActionActWindow(ActionMixin, ModelSQL, ModelView): action=action.rec_name)) @classmethod - def check_domain(cls, actions): + def check_domain(cls, actions, field_names=None): "Check domain and search_value" + if field_names and not (field_names & {'domain', 'search_value'}): + return for action in actions: for domain in (action.domain, action.search_value): if not domain: @@ -859,8 +872,10 @@ class ActionActWindow(ActionMixin, ModelSQL, ModelView): action=action.rec_name)) from exception @classmethod - def check_context(cls, actions): + def check_context(cls, actions, field_names=None): "Check context" + if field_names and 'context' not in field_names: + return for action in actions: if action.context: try: @@ -995,12 +1010,14 @@ class ActionActWindowDomain( return False @classmethod - def validate(cls, actions): - super(ActionActWindowDomain, cls).validate(actions) - cls.check_domain(actions) + def validate_fields(cls, actions, field_names): + super().validate_fields(actions, field_names) + cls.check_domain(actions, field_names) @classmethod - def check_domain(cls, actions): + def check_domain(cls, actions, field_names=None): + if field_names and 'domain' not in field_names: + return for action in actions: if not action.domain: continue diff --git a/trytond/ir/email_.py b/trytond/ir/email_.py index b54114483d..dc3653a5f7 100644 --- a/trytond/ir/email_.py +++ b/trytond/ir/email_.py @@ -344,59 +344,73 @@ class EmailTemplate(ModelSQL, ModelView): return self.model.model @classmethod - def validate(cls, templates): - super().validate(templates) + def validate_fields(cls, templates, field_names): + super().validate_fields(templates, field_names) + cls.check_subject(templates, field_names) + cls.check_body(templates, field_names) + cls.check_fields_pyson(templates, field_names) + + @classmethod + def check_subject(cls, templates, field_names=None): + if field_names and 'subject' not in field_names: + return for template in templates: - template.check_subject() - template.check_body() - template.check_fields_pyson() - - def check_subject(self): - if not self.subject: - return - try: - TextTemplate(self.subject) - except Exception as exception: - raise EmailTemplateError( - gettext('ir.msg_email_template_invalid_subject', - template=self.rec_name, - exception=exception)) from exception - - def check_body(self): - if not self.body: - return - try: - TextTemplate(self.body) - except Exception as exception: - raise EmailTemplateError( - gettext('ir.msg_email_template_invalid_body', - template=self.rec_name, - exception=exception)) from exception - - def check_fields_pyson(self): - encoder = PYSONDecoder(noeval=True) - for field in [ - 'recipients_pyson', - 'recipients_secondary_pyson', - 'recipients_hidden_pyson', - ]: - value = getattr(self, field) - if not value: + if not template.subject: continue try: - pyson = encoder.decode(value) + TextTemplate(template.subject) except Exception as exception: - raise EmailTemplateError( - gettext('ir.msg_email_template_invalid_field_pyson', - template=self.rec_name, - field=self.__names__(field)['field'], + raise EmailTemplateError(gettext( + 'ir.msg_email_template_invalid_subject', + template=template.rec_name, exception=exception)) from exception - if not isinstance(pyson, list) and pyson.types() != {list}: - raise EmailTemplateError( - gettext('ir.msg_email_template_invalid_field_pyson_type', - template=self.rec_name, - field=self.__names__(field)['field'], - )) + + @classmethod + def check_body(self, templates, field_names=None): + if field_names and 'body' not in field_names: + return + for template in templates: + if not template.body: + continue + try: + TextTemplate(template.body) + except Exception as exception: + raise EmailTemplateError(gettext( + 'ir.msg_email_template_invalid_body', + template=template.rec_name, + exception=exception)) from exception + + @classmethod + def check_fields_pyson(cls, templates, field_names=None): + pyson_fields = { + 'recipients_pyson', + 'recipients_secondary_pyson', + 'recipients_hidden_pyson', + } + if field_names: + pyson_fields &= field_names + if not pyson_fields: + return + encoder = PYSONDecoder(noeval=True) + for template in templates: + for field in pyson_fields: + value = getattr(template, field) + if not value: + continue + try: + pyson = encoder.decode(value) + except Exception as exception: + raise EmailTemplateError( + gettext('ir.msg_email_template_invalid_field_pyson', + template=template.rec_name, + field=cls.__names__(field)['field'], + exception=exception)) from exception + if not isinstance(pyson, list) and pyson.types() != {list}: + raise EmailTemplateError(gettext( + 'ir.msg_email_template_invalid_field_pyson_type', + template=template.rec_name, + field=cls.__names__(field)['field'], + )) def get(self, record): pool = Pool() diff --git a/trytond/ir/lang.py b/trytond/ir/lang.py index 3a50823581..d6e61e3599 100644 --- a/trytond/ir/lang.py +++ b/trytond/ir/lang.py @@ -230,17 +230,19 @@ class Lang(DeactivableMixin, ModelSQL, ModelView): ])) @classmethod - def validate(cls, languages): - super(Lang, cls).validate(languages) - cls.check_grouping(languages) - cls.check_date(languages) + def validate_fields(cls, languages, field_names): + super().validate_fields(languages, field_names) + cls.check_grouping(languages, field_names) + cls.check_date(languages, field_names) cls.check_translatable(languages) @classmethod - def check_grouping(cls, langs): + def check_grouping(cls, langs, fields_names=None): ''' Check if grouping is list of numbers ''' + if fields_names and not (fields_names & {'grouping', 'mon_grouping'}): + return for lang in langs: for grouping in [lang.grouping, lang.mon_grouping]: try: @@ -255,10 +257,12 @@ class Lang(DeactivableMixin, ModelSQL, ModelView): language=lang.rec_name)) @classmethod - def check_date(cls, langs): + def check_date(cls, langs, field_names=None): ''' Check the date format ''' + if field_names and 'date' not in field_names: + return for lang in langs: date = lang.date try: @@ -286,9 +290,11 @@ class Lang(DeactivableMixin, ModelSQL, ModelView): language=lang.rec_name)) @classmethod - def check_translatable(cls, langs): + def check_translatable(cls, langs, field_names=None): pool = Pool() Config = pool.get('ir.configuration') + if field_names and 'translatable' not in field_names: + return # Skip check for root because when languages are created from XML file, # translatable is not yet set. if Transaction().user == 0: diff --git a/trytond/ir/model.py b/trytond/ir/model.py index d31c6f8bad..7f1ae04e47 100644 --- a/trytond/ir/model.py +++ b/trytond/ir/model.py @@ -1035,12 +1035,14 @@ class ModelButtonRule(ModelSQL, ModelView): return 1 @classmethod - def validate(cls, rules): - super(ModelButtonRule, cls).validate(rules) - cls.check_condition(rules) + def validate_fields(cls, rules, field_names): + super().validate_fields(rules, field_names) + cls.check_condition(rules, field_names) @classmethod - def check_condition(cls, rules): + def check_condition(cls, rules, field_names=None): + if field_names and 'condition' not in field_names: + return for rule in rules: if not rule.condition: continue diff --git a/trytond/ir/rule.py b/trytond/ir/rule.py index 879d3b0ca6..e833f6ab01 100644 --- a/trytond/ir/rule.py +++ b/trytond/ir/rule.py @@ -110,12 +110,14 @@ class Rule(ModelSQL, ModelView): modes = {'read', 'write', 'create', 'delete'} @classmethod - def validate(cls, rules): - super(Rule, cls).validate(rules) - cls.check_domain(rules) + def validate_fields(cls, rules, field_names): + super().validate_fields(rules, field_names) + cls.check_domain(rules, field_names) @classmethod - def check_domain(cls, rules): + def check_domain(cls, rules, field_names=None): + if field_names and 'domain' not in field_names: + return ctx = cls._get_context() for rule in rules: try: diff --git a/trytond/ir/sequence.py b/trytond/ir/sequence.py index b2b748a477..f192e95514 100644 --- a/trytond/ir/sequence.py +++ b/trytond/ir/sequence.py @@ -225,13 +225,19 @@ class Sequence(DeactivableMixin, ModelSQL, ModelView): @classmethod def validate(cls, sequences): - super(Sequence, cls).validate(sequences) - cls.check_affixes(sequences) + super().validate(sequences) cls.check_last_timestamp(sequences) @classmethod - def check_affixes(cls, sequences): + def validate_fields(cls, sequences, field_names): + super().validate_fields(sequences, field_names) + cls.check_affixes(sequences, field_names) + + @classmethod + def check_affixes(cls, sequences, field_names=None): "Check prefix and suffix" + if field_names and not (field_names & {'prefix', 'suffix'}): + return for sequence in sequences: for affix, error_message in [ (sequence.prefix, 'msg_sequence_invalid_prefix'), diff --git a/trytond/ir/trigger.py b/trytond/ir/trigger.py index 3137e01ecb..4f8ca22727 100644 --- a/trytond/ir/trigger.py +++ b/trytond/ir/trigger.py @@ -109,15 +109,17 @@ class Trigger(DeactivableMixin, ModelSQL, ModelView): table_h.drop_column('action_function') @classmethod - def validate(cls, triggers): - super(Trigger, cls).validate(triggers) - cls.check_condition(triggers) + def validate_fields(cls, triggers, field_names): + super().validate_fields(triggers, field_names) + cls.check_condition(triggers, field_names) @classmethod - def check_condition(cls, triggers): + def check_condition(cls, triggers, field_names=None): ''' Check condition ''' + if field_names and 'condition' not in field_names: + return for trigger in triggers: try: PYSONDecoder(noeval=True).decode(trigger.condition) diff --git a/trytond/model/dictschema.py b/trytond/model/dictschema.py index 67d92ba391..3bae43539d 100644 --- a/trytond/model/dictschema.py +++ b/trytond/model/dictschema.py @@ -108,13 +108,15 @@ class DictSchemaMixin(object): self.name = slugify(self.string.lower(), hyphenate='_') @classmethod - def validate(cls, schemas): - super(DictSchemaMixin, cls).validate(schemas) - cls.check_domain(schemas) - cls.check_selection(schemas) + def validate_fields(cls, schemas, field_names): + super().validate_fields(schemas, field_names) + cls.check_domain(schemas, field_names) + cls.check_selection(schemas, field_names) @classmethod - def check_domain(cls, schemas): + def check_domain(cls, schemas, field_names=None): + if field_names and 'domain' not in field_names: + return for schema in schemas: if not schema.domain: continue @@ -130,7 +132,10 @@ class DictSchemaMixin(object): schema=schema.rec_name)) @classmethod - def check_selection(cls, schemas): + def check_selection(cls, schemas, field_names=None): + if field_names and not (field_names & { + 'type_', 'selection_json', 'help_selection_json'}): + return for schema in schemas: if schema.type_ not in {'selection', 'multiselection'}: continue diff --git a/trytond/model/modelstorage.py b/trytond/model/modelstorage.py index 94a39c1ed9..9024faf791 100644 --- a/trytond/model/modelstorage.py +++ b/trytond/model/modelstorage.py @@ -1134,6 +1134,10 @@ class ModelStorage(Model): def validate(cls, records): pass + @classmethod + def validate_fields(cls, records, field_names): + pass + @classmethod @without_check_access def _validate(cls, records, field_names=None): @@ -1314,14 +1318,16 @@ class ModelStorage(Model): msg, domain=(invalid_domain, field_def)) raise DomainValidationError(msg) - field_names = set(field_names or []) + if field_names is None: + field_names = cls._fields.keys() + elif not isinstance(field_names, set): + field_names = set(field_names) function_fields = {name for name, field in cls._fields.items() if isinstance(field, fields.Function)} ctx_pref['active_test'] = False with Transaction().set_context(ctx_pref): for field_name, field in cls._fields.items(): - if (field_names - and field_name not in field_names + if (field_name not in field_names and not (field.validation_depends & field_names) and not (field.validation_depends & function_fields)): continue @@ -1504,6 +1510,7 @@ class ModelStorage(Model): record.pre_validate() cls.validate(records) + cls.validate_fields(records, field_names) @classmethod def _clean_defaults(cls, defaults): diff --git a/trytond/model/tree.py b/trytond/model/tree.py index af1096544a..ca3617ccee 100644 --- a/trytond/model/tree.py +++ b/trytond/model/tree.py @@ -87,18 +87,21 @@ def tree(parent='parent', name='name', separator=None): return domain @classmethod - def validate(cls, records): - super(TreeMixin, cls).validate(records) - cls.check_recursion(records) + def validate_fields(cls, records, field_names): + super().validate_fields(records, field_names) + cls.check_recursion(records, field_names) @classmethod - def check_recursion(cls, records): + def check_recursion(cls, records, field_names=None): ''' Function that checks if there is no recursion in the tree composed with parent as parent field name. ''' if hasattr(super(TreeMixin, cls), 'check_recursion'): - super(TreeMixin, cls).check_recursion(records) + super(TreeMixin, cls).check_recursion(records, field_names) + + if field_names and parent not in field_names: + return parent_type = cls._fields[parent]._type