Commit 73ab30a1 authored by Adrien Di Mascio's avatar Adrien Di Mascio
Browse files

[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.

--HG--
branch : stable
parent 929984f017e6
......@@ -471,14 +471,14 @@ class NativeSQLSource(SQLAdapterMixIn, AbstractSource):
# 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 @@ class NativeSQLSource(SQLAdapterMixIn, AbstractSource):
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):
......
......@@ -72,28 +72,23 @@ class BytesFileSystemStorage(Storage):
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 @@ class BytesFileSystemStorage(Storage):
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)
......
......@@ -39,7 +39,6 @@ class DummyAfterHook(Hook):
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 @@ class StorageTC(CubicWebTC):
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 @@ class StorageTC(CubicWebTC):
{'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()
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment