# HG changeset patch # User Adrien Di Mascio # Date 1270190581 -7200 # Node ID 88b5ca8da928b9ba00b9537d2f186464df7186aa # Parent 929984f017e675dcd9a1499d295b5520e9bc4b7f [storages] fix fs_importing side-effect on entity.data When creating a new File object, if fs_importing is set, we want entity.data to be the file content instead of the filepath for the rest of the transaction. (see test_bfss_fs_importing_transparency) for test implementation To make this possible, the storage hooks (entity_added / entity_updated) must return the correct value to set in the entity dict. diff -r 929984f017e6 -r 88b5ca8da928 server/sources/native.py --- a/server/sources/native.py Thu Apr 01 11:48:18 2010 +0200 +++ b/server/sources/native.py Fri Apr 02 08:43:01 2010 +0200 @@ -471,14 +471,14 @@ # on the filesystem. To make the entity.data usage absolutely # transparent, we'll have to reset entity.data to its binary # value once the SQL query will be executed - orig_values = {} + restore_values = {} etype = entity.__regid__ for attr, storage in self._storages.get(etype, {}).items(): try: if attr in entity.edited_attributes: - orig_values[attr] = entity[attr] handler = getattr(storage, 'entity_%s' % event) - handler(entity, attr) + real_value = handler(entity, attr) + restore_values[attr] = real_value except AttributeError: assert event == 'deleted' getattr(storage, 'entity_deleted')(entity, attr) @@ -486,7 +486,7 @@ yield # 2/ execute the source's instructions finally: # 3/ restore original values - for attr, value in orig_values.items(): + for attr, value in restore_values.items(): entity[attr] = value def add_entity(self, session, entity): diff -r 929984f017e6 -r 88b5ca8da928 server/sources/storages.py --- a/server/sources/storages.py Thu Apr 01 11:48:18 2010 +0200 +++ b/server/sources/storages.py Fri Apr 02 08:43:01 2010 +0200 @@ -72,28 +72,23 @@ def entity_added(self, entity, attr): """an entity using this storage for attr has been added""" - if not entity._cw.transaction_data.get('fs_importing'): - try: - value = entity.pop(attr) - except KeyError: - pass - else: - fpath = self.new_fs_path(entity, attr) - # bytes storage used to store file's path - entity[attr] = Binary(fpath) - file(fpath, 'w').write(value.getvalue()) - AddFileOp(entity._cw, filepath=fpath) - # else entity[attr] is expected to be an already existant file path + if entity._cw.transaction_data.get('fs_importing'): + binary = Binary(file(entity[attr].getvalue()).read()) + else: + binary = entity.pop(attr) + fpath = self.new_fs_path(entity, attr) + # bytes storage used to store file's path + entity[attr] = Binary(fpath) + file(fpath, 'w').write(binary.getvalue()) + AddFileOp(entity._cw, filepath=fpath) + return binary def entity_updated(self, entity, attr): """an entity using this storage for attr has been updatded""" - try: - value = entity.pop(attr) - except KeyError: - pass - else: - fpath = self.current_fs_path(entity, attr) - UpdateFileOp(entity._cw, filepath=fpath, filedata=value.getvalue()) + binary = entity.pop(attr) + fpath = self.current_fs_path(entity, attr) + UpdateFileOp(entity._cw, filepath=fpath, filedata=binary.getvalue()) + return binary def entity_deleted(self, entity, attr): """an entity using this storage for attr has been deleted""" @@ -110,8 +105,10 @@ cu = sysource.doexec(entity._cw, 'SELECT cw_%s FROM cw_%s WHERE cw_eid=%s' % ( attr, entity.__regid__, entity.eid)) - BINARY = sysource.dbhelper.dbapi_module.BINARY - return sysource._process_value(cu.fetchone()[0], [None, BINARY], + rawvalue = cu.fetchone()[0] + if rawvalue is None: # no previous value + return self.new_fs_path(entity, attr) + return sysource._process_value(rawvalue, cu.description[0], binarywrap=str) diff -r 929984f017e6 -r 88b5ca8da928 server/test/unittest_storage.py --- a/server/test/unittest_storage.py Thu Apr 01 11:48:18 2010 +0200 +++ b/server/test/unittest_storage.py Fri Apr 02 08:43:01 2010 +0200 @@ -39,7 +39,6 @@ oldvalue = self._cw.transaction_data['orig_file_value'] assert oldvalue == self.entity.data.getvalue() - class StorageTC(CubicWebTC): def setup_database(self): @@ -88,11 +87,12 @@ def test_bfss_fs_importing_doesnt_touch_path(self): self.session.transaction_data['fs_importing'] = True - f1 = self.session.create_entity('File', data=Binary('/the/path'), + filepath = osp.abspath(__file__) + f1 = self.session.create_entity('File', data=Binary(filepath), data_format=u'text/plain', data_name=u'foo') fspath = self.execute('Any fspath(D) WHERE F eid %(f)s, F data D', {'f': f1.eid})[0][0] - self.assertEquals(fspath.getvalue(), '/the/path') + self.assertEquals(fspath.getvalue(), filepath) def test_source_storage_transparency(self): with self.temporary_appobjects(DummyBeforeHook, DummyAfterHook): @@ -156,5 +156,29 @@ {'x': f1.eid}, 'x') self.assertEquals(str(ex), 'UPPER can not be called on mapped attribute') + + def test_bfss_fs_importing_transparency(self): + self.session.transaction_data['fs_importing'] = True + filepath = osp.abspath(__file__) + f1 = self.session.create_entity('File', data=Binary(filepath), + data_format=u'text/plain', data_name=u'foo') + self.assertEquals(f1.data.getvalue(), file(filepath).read(), + 'files content differ') + + + def test_bfss_update_with_existing_data(self): + # use self.session to use server-side cache + f1 = self.session.create_entity('File', data=Binary('some data'), + data_format=u'text/plain', data_name=u'foo') + # NOTE: do not use set_attributes() which would automatically + # update f1's local dict. We want the pure rql version to work + self.execute('SET F data %(d)s WHERE F eid %(f)s', + {'d': Binary('some other data'), 'f': f1.eid}) + self.assertEquals(f1.data.getvalue(), 'some other data') + self.commit() + f2 = self.entity('Any F WHERE F eid %(f)s, F is File', {'f': f1.eid}) + self.assertEquals(f2.data.getvalue(), 'some other data') + + if __name__ == '__main__': unittest_main()