[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.
--- 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):
--- 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)
--- 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()