From 7ca7f4c6f0e1745eb4247d001f7d2619c9767ac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Fri, 29 Jul 2016 10:51:53 +0200 Subject: [PATCH] Allow to store Binary field in filestore Two options are added on Binary field to define the column to use to store the filestore id and the prefix to use. Those options are used for Attachment and are configurable with conservative default values. issue5692 review25471003 --- CHANGELOG | 2 + doc/ref/models/fields.rst | 20 +++++++++- doc/topics/configuration.rst | 18 +++++++++ trytond/ir/attachment.py | 56 ++++++++++----------------- trytond/model/fields/binary.py | 66 +++++++++++++++++++++++++++++--- trytond/model/fields/field.py | 2 +- trytond/model/fields/function.py | 3 +- trytond/model/modelsql.py | 22 ++++++----- trytond/tests/__init__.py | 1 + trytond/tests/test.py | 9 ++++- trytond/tests/test_fields.py | 36 +++++++++++++++++ 11 files changed, 178 insertions(+), 57 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 18563250e6..9d19ac23ed 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,5 @@ +* Add option to store Attachment in database +* Allow to store Binary field in filestore * Add filestore module * Allow None in Greater/Less PYSON * Add option to set admin password with trytond-admin diff --git a/doc/ref/models/fields.rst b/doc/ref/models/fields.rst index 8b7bc01604..d94a1a15ea 100644 --- a/doc/ref/models/fields.rst +++ b/doc/ref/models/fields.rst @@ -171,7 +171,8 @@ Instance methods: .. method:: Field.sql_type() Return the namedtuple('SQLType', 'base type') which defines the SQL type to - use for creation and casting. + use for creation and casting. Or `None` if the field is not stored in the + database. .. method:: Field.sql_column(table) @@ -384,7 +385,7 @@ A binary field. It will be represented in Python by a ``bytes`` instance. separated by a dot and its value is the string `size` then the read value is the size instead of the content. -:class:`Binary` has one extra optional argument: +:class:`Binary` has three extra optional arguments: .. attribute:: Binary.filename @@ -393,6 +394,21 @@ A binary field. It will be represented in Python by a ``bytes`` instance. filename is hidden, and the "Open" button is hidden when the widget is set to "image"). +.. attribute:: Binary.file_id + + Name of the field that holds the `FileStore` identifier. Default value is + `None` which means the data is stored in the database. The field must be on + the same table and accept `char` values. + +.. warning:: + Switching from database to file-store is supported transparently. But + switching from file-store to database is not supported without manually + upload to the database all the files. + +.. attribute:: Binary.store_prefix + + The prefix to use with the `FileStore`. Default value is `None` which means + the database name is used. Selection --------- diff --git a/doc/topics/configuration.rst b/doc/topics/configuration.rst index 012d01707b..4bf43e013b 100644 --- a/doc/topics/configuration.rst +++ b/doc/topics/configuration.rst @@ -229,6 +229,24 @@ The parameters for `unoconv`. Default: `pipe,name=trytond;urp;StarOffice.ComponentContext` +attachment +---------- + +Defines how to store the attachments + +filestore +~~~~~~~~~ + +A boolean value to store attachment in the :ref:`FileStore `. + +Default: `True` + +store_prefix +~~~~~~~~~~~~ + +The prefix to use with the `FileStore`. + +Default: `None` .. _JSON-RPC: http://en.wikipedia.org/wiki/JSON-RPC .. _XML-RPC: http://en.wikipedia.org/wiki/XML-RPC diff --git a/trytond/ir/attachment.py b/trytond/ir/attachment.py index 1811e06789..7ca225fe15 100644 --- a/trytond/ir/attachment.py +++ b/trytond/ir/attachment.py @@ -8,7 +8,7 @@ from .. import backend from ..transaction import Transaction from ..pyson import Eval from .resource import ResourceMixin -from ..filestore import filestore +from ..config import config __all__ = [ 'Attachment', @@ -21,6 +21,13 @@ def firstline(description): except StopIteration: return '' +if config.getboolean('attachment', 'filestore', default=True): + file_id = 'file_id' + store_prefix = config.get('attachment', 'store_prefix', default=None) +else: + file_id = None + store_prefix = None + class Attachment(ResourceMixin, ModelSQL, ModelView): "Attachment" @@ -30,18 +37,20 @@ class Attachment(ResourceMixin, ModelSQL, ModelView): ('data', 'Data'), ('link', 'Link'), ], 'Type', required=True) - data = fields.Function(fields.Binary('Data', filename='name', states={ - 'invisible': Eval('type') != 'data', - }, depends=['type']), 'get_data', setter='set_data') description = fields.Text('Description') summary = fields.Function(fields.Char('Summary'), 'on_change_with_summary') link = fields.Char('Link', states={ 'invisible': Eval('type') != 'link', }, depends=['type']) + data = fields.Binary('Data', filename='name', + file_id=file_id, store_prefix=store_prefix, + states={ + 'invisible': Eval('type') != 'data', + }, depends=['type']) file_id = fields.Char('File ID', readonly=True) data_size = fields.Function(fields.Integer('Data size', states={ 'invisible': Eval('type') != 'data', - }, depends=['type']), 'get_data') + }, depends=['type']), 'get_size') @classmethod def __setup__(cls): @@ -95,37 +104,12 @@ class Attachment(ResourceMixin, ModelSQL, ModelView): def default_type(): return 'data' - def get_data(self, name): - db_name = Transaction().database.name - format_ = Transaction().context.get( - '%s.%s' % (self.__name__, name), '') - value = None - if name == 'data_size' or format_ == 'size': - value = 0 - if self.file_id: - if name == 'data_size' or format_ == 'size': - try: - value = filestore.size(self.file_id, prefix=db_name) - except OSError: - pass - else: - try: - value = fields.Binary.cast( - filestore.get(self.file_id, prefix=db_name)) - except IOError: - pass - return value - - @classmethod - def set_data(cls, attachments, name, value): - if value is None: - return - transaction = Transaction() - db_name = transaction.database.name - file_id = filestore.set(value, prefix=db_name) - cls.write(attachments, { - 'file_id': file_id, - }) + def get_size(self, name): + with Transaction().set_context({ + '%s.%s' % (self.__name__, name): 'size', + }): + record = self.__class__(self.id) + return record.data @fields.depends('description') def on_change_with_summary(self, name=None): diff --git a/trytond/model/fields/binary.py b/trytond/model/fields/binary.py index 43795d4943..4dba060978 100644 --- a/trytond/model/fields/binary.py +++ b/trytond/model/fields/binary.py @@ -1,10 +1,12 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. -from sql import Query, Expression +from sql import Query, Expression, Column, Null from .field import Field, SQLType from ...transaction import Transaction from ... import backend +from ...tools import grouped_slice, reduce_ids +from ...filestore import filestore class Binary(Field): @@ -16,21 +18,22 @@ class Binary(Field): def __init__(self, string='', help='', required=False, readonly=False, domain=None, states=None, select=False, on_change=None, - on_change_with=None, depends=None, filename=None, context=None, - loading='lazy'): + on_change_with=None, depends=None, context=None, loading='lazy', + filename=None, file_id=None, store_prefix=None): if filename is not None: self.filename = filename if depends is None: depends = [filename] else: depends.append(filename) + self.file_id = file_id + self.store_prefix = store_prefix super(Binary, self).__init__(string=string, help=help, required=required, readonly=readonly, domain=domain, states=states, select=select, on_change=on_change, on_change_with=on_change_with, depends=depends, context=context, loading=loading) - @classmethod - def get(cls, ids, model, name, values=None): + def get(self, ids, model, name, values=None): ''' Convert the binary value into ``bytes`` @@ -42,15 +45,45 @@ class Binary(Field): ''' if values is None: values = {} + transaction = Transaction() res = {} - converter = cls.cast + converter = self.cast default = None format_ = Transaction().context.get( '%s.%s' % (model.__name__, name), '') if format_ == 'size': converter = len default = 0 + + if self.file_id: + table = model.__table__() + cursor = transaction.connection.cursor() + + prefix = self.store_prefix + if prefix is None: + prefix = transaction.database.name + + if format_ == 'size': + store_func = filestore.size + else: + def store_func(id, prefix): + return self.cast(filestore.get(id, prefix=prefix)) + + for sub_ids in grouped_slice(ids): + cursor.execute(*table.select( + table.id, Column(table, self.file_id), + where=reduce_ids(table.id, sub_ids) + & (Column(table, self.file_id) != Null) + & (Column(table, self.file_id) != ''))) + for record_id, file_id in cursor: + try: + res[record_id] = store_func(file_id, prefix) + except (IOError, OSError): + pass + for i in values: + if i['id'] in res: + continue value = i[name] if value: if isinstance(value, unicode): @@ -63,6 +96,27 @@ class Binary(Field): res.setdefault(i, default) return res + def set(self, Model, name, ids, value, *args): + transaction = Transaction() + table = Model.__table__() + cursor = transaction.connection.cursor() + + prefix = self.store_prefix + if prefix is None: + prefix = transaction.database.name + + args = iter((ids, value) + args) + for ids, value in zip(args, args): + if self.file_id: + columns = [Column(table, self.file_id), Column(table, name)] + values = [ + filestore.set(value, prefix) if value else None, None] + else: + columns = [Column(table, name)] + values = [self.sql_format(value)] + cursor.execute(*table.update(columns, values, + where=reduce_ids(table.id, ids))) + @staticmethod def sql_format(value): if isinstance(value, (Query, Expression)): diff --git a/trytond/model/fields/field.py b/trytond/model/fields/field.py index aff20856ee..34f17b1714 100644 --- a/trytond/model/fields/field.py +++ b/trytond/model/fields/field.py @@ -244,7 +244,7 @@ class Field(object): return value def sql_type(self): - raise NotImplementedError + return None def sql_column(self, table): return Column(table, self.name) diff --git a/trytond/model/fields/function.py b/trytond/model/fields/function.py index e5a48af2ff..08a08e4e41 100644 --- a/trytond/model/fields/function.py +++ b/trytond/model/fields/function.py @@ -58,9 +58,8 @@ class Function(Field): return setattr(self._field, name, value) - @property def sql_type(self): - raise AttributeError + return None def convert_domain(self, domain, tables, Model): name, operator, value = domain[:3] diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py index 50482deb17..9f5e046380 100644 --- a/trytond/model/modelsql.py +++ b/trytond/model/modelsql.py @@ -145,10 +145,10 @@ class ModelSQL(ModelStorage): for field_name, field in cls._fields.iteritems(): if field_name == 'id': continue - default_fun = None - if hasattr(field, 'set'): - continue sql_type = field.sql_type() + if not sql_type: + continue + default_fun = None if field_name in cls._defaults: default_fun = cls._defaults[field_name] @@ -199,6 +199,10 @@ class ModelSQL(ModelStorage): field_name, action=field.select and 'add' or 'remove') required = field.required + # Do not set 'NOT NULL' for Binary field stored in the filestore + # as the database column will be left empty. + if isinstance(field, fields.Binary) and field.file_id: + required = False table.not_null_action( field_name, action=required and 'add' or 'remove') @@ -228,7 +232,7 @@ class ModelSQL(ModelStorage): cursor.execute(*history_table.select(history_table.id)) if not cursor.fetchone(): columns = [n for n, f in cls._fields.iteritems() - if not hasattr(f, 'set')] + if f.sql_type()] cursor.execute(*history_table.insert( [Column(history_table, c) for c in columns], sql_table.select(*(Column(sql_table, c) @@ -274,7 +278,7 @@ class ModelSQL(ModelStorage): field = cls._fields[field_name] # Check required fields if (field.required - and not hasattr(field, 'set') + and field.sql_type() and field_name not in ('create_uid', 'create_date')): if values.get(field_name) is None: cls.raise_user_error('required_field', @@ -354,7 +358,7 @@ class ModelSQL(ModelStorage): 'write_date': cls.write_date, } for fname, field in sorted(fields.iteritems()): - if hasattr(field, 'set'): + if not field.sql_type(): continue columns.append(Column(table, fname)) hcolumns.append(Column(history, fname)) @@ -384,7 +388,7 @@ class ModelSQL(ModelStorage): columns = [] hcolumns = [] fnames = sorted(n for n, f in cls._fields.iteritems() - if not hasattr(f, 'set')) + if f.sql_type()) for fname in fnames: columns.append(Column(table, fname)) if fname == 'write_uid': @@ -663,7 +667,7 @@ class ModelSQL(ModelStorage): columns = [] for f in fields_names + fields_related.keys() + datetime_fields: field = cls._fields.get(f) - if field and not hasattr(field, 'set'): + if field and field.sql_type(): columns.append(field.sql_column(table).as_(f)) elif f == '_timestamp' and not table_query: sql_type = fields.Char('timestamp').sql_type().base @@ -715,7 +719,7 @@ class ModelSQL(ModelStorage): if field == '_timestamp': continue if (getattr(cls._fields[field], 'translate', False) - and not hasattr(field, 'set')): + and not hasattr(field, 'get')): translations = Translation.get_ids(cls.__name__ + ',' + field, 'model', Transaction().language, ids) for row in result: diff --git a/trytond/tests/__init__.py b/trytond/tests/__init__.py index 2dd5214d96..968bcfe163 100644 --- a/trytond/tests/__init__.py +++ b/trytond/tests/__init__.py @@ -101,6 +101,7 @@ def register(): Binary, BinaryDefault, BinaryRequired, + BinaryFileStorage, Model, Singleton, URLObject, diff --git a/trytond/tests/test.py b/trytond/tests/test.py index 62a03a6fd9..b7e9e3beec 100644 --- a/trytond/tests/test.py +++ b/trytond/tests/test.py @@ -35,7 +35,7 @@ __all__ = [ 'Property', 'Selection', 'SelectionRequired', 'DictSchema', 'Dict', 'DictDefault', 'DictRequired', - 'Binary', 'BinaryDefault', 'BinaryRequired', + 'Binary', 'BinaryDefault', 'BinaryRequired', 'BinaryFileStorage', 'Many2OneDomainValidation', 'Many2OneTarget', 'Many2OneOrderBy', 'Many2OneSearch', 'Many2OneTree', 'Many2OneMPTT', ] @@ -727,6 +727,13 @@ class BinaryRequired(ModelSQL): binary = fields.Binary('Binary Required', required=True) +class BinaryFileStorage(ModelSQL): + "Binary in FileStorage" + __name__ = 'test.binary_filestorage' + binary = fields.Binary('Binary', file_id='binary_id') + binary_id = fields.Char('Binary ID') + + class Many2OneTarget(ModelSQL): "Many2One Domain Validation Target" __name__ = 'test.many2one_target' diff --git a/trytond/tests/test_fields.py b/trytond/tests/test_fields.py index 10b0b78896..5a99f039d4 100644 --- a/trytond/tests/test_fields.py +++ b/trytond/tests/test_fields.py @@ -11,12 +11,16 @@ except ImportError: sys.modules['cdecimal'] = decimal import unittest import datetime +import shutil +import tempfile + from decimal import Decimal from trytond.tests.test_tryton import install_module, with_transaction from trytond.transaction import Transaction from trytond.exceptions import UserError from trytond.model import fields from trytond.pool import Pool +from trytond.config import config class FieldsTestCase(unittest.TestCase): @@ -26,6 +30,13 @@ class FieldsTestCase(unittest.TestCase): def setUpClass(cls): install_module('tests') + def setUp(self): + path = config.get('database', 'path') + dtemp = tempfile.mkdtemp() + config.set('database', 'path', dtemp) + self.addCleanup(config.set, 'database', 'path', path) + self.addCleanup(shutil.rmtree, dtemp) + @with_transaction() def test_boolean(self): 'Test Boolean' @@ -3286,6 +3297,31 @@ class FieldsTestCase(unittest.TestCase): self.assertRaises(UserError, BinaryRequired.create, [{'binary': fields.Binary.cast(b'')}]) + @with_transaction() + def test_binary_filestorage(self): + "Test Binary FileStorage" + pool = Pool() + Binary = pool.get('test.binary_filestorage') + transaction = Transaction() + + bin1, = Binary.create([{ + 'binary': fields.Binary.cast(b'foo'), + }]) + self.assertEqual(bin1.binary, fields.Binary.cast(b'foo')) + self.assertTrue(bin1.binary_id) + + Binary.write([bin1], {'binary': fields.Binary.cast(b'bar')}) + self.assertEqual(bin1.binary, fields.Binary.cast(b'bar')) + + with transaction.set_context( + {'test.binary_filestorage.binary': 'size'}): + bin1_size = Binary(bin1.id) + self.assertEqual(bin1_size.binary, len(b'bar')) + + Binary.write([bin1], {'binary': None}) + self.assertEqual(bin1.binary, None) + self.assertEqual(bin1.binary_id, None) + @with_transaction() def test_many2one(self): 'Test Many2One'