[bfss] Fix update of BFSS attribute to None (close #1875289) oldstable
authorPierre-Yves David <pierre-yves.david@logilab.fr>
Tue, 26 Jul 2011 16:33:06 +0200
brancholdstable
changeset 7694 bd56a29acaa8
parent 7693 e2f75311d7be
child 7695 2f6e37661cf6
child 7703 2364087528e7
[bfss] Fix update of BFSS attribute to None (close #1875289) The current code assume attr value is alway a valide Binary value. But when the attr is optional, the value might be None. The change handle this case.
server/sources/storages.py
server/test/data/schema.py
server/test/unittest_storage.py
--- a/server/sources/storages.py	Tue Jul 26 16:05:36 2011 +0200
+++ b/server/sources/storages.py	Tue Jul 26 16:33:06 2011 +0200
@@ -148,6 +148,7 @@
             # We do not need to create it but we need to fetch the content of
             # the file as the actual content of the attribute
             fpath = entity.cw_edited[attr].getvalue()
+            assert fpath is not None
             binary = Binary(file(fpath, 'rb').read())
         else:
             # We must store the content of the attributes
@@ -160,17 +161,23 @@
             #
             # fetch the current attribute value in memory
             binary = entity.cw_edited.pop(attr)
-            # Get filename for it
-            fpath = self.new_fs_path(entity, attr)
-            assert not osp.exists(fpath)
-            # write attribute value on disk
-            file(fpath, 'wb').write(binary.getvalue())
-            # Mark the new file as added during the transaction.
-            # The file will be removed on rollback
-            AddFileOp.get_instance(entity._cw).add_data(fpath)
+            if binary is None:
+                fpath = None
+            else:
+                # Get filename for it
+                fpath = self.new_fs_path(entity, attr)
+                assert not osp.exists(fpath)
+                # write attribute value on disk
+                file(fpath, 'wb').write(binary.getvalue())
+                # Mark the new file as added during the transaction.
+                # The file will be removed on rollback
+                AddFileOp.get_instance(entity._cw).add_data(fpath)
         if oldpath != fpath:
             # register the new location for the file.
-            entity.cw_edited.edited_attribute(attr, Binary(fpath))
+            if fpath is None:
+                entity.cw_edited.edited_attribute(attr, None)
+            else:
+                entity.cw_edited.edited_attribute(attr, Binary(fpath))
             # Mark the old file as useless so the file will be removed at
             # commit.
             if oldpath is not None:
--- a/server/test/data/schema.py	Tue Jul 26 16:05:36 2011 +0200
+++ b/server/test/data/schema.py	Tue Jul 26 16:33:06 2011 +0200
@@ -18,12 +18,15 @@
 
 from yams.buildobjs import (EntityType, RelationType, RelationDefinition,
                             SubjectRelation, RichString, String, Int, Float,
-                            Boolean, Datetime, TZDatetime)
+                            Boolean, Datetime, TZDatetime, Bytes)
 from yams.constraints import SizeConstraint
 from cubicweb.schema import (WorkflowableEntityType,
                              RQLConstraint, RQLUniqueConstraint,
                              ERQLExpression, RRQLExpression)
 
+class BFSSTestable(EntityType):
+    opt_attr = Bytes()
+
 class Affaire(WorkflowableEntityType):
     __permissions__ = {
         'read':   ('managers',
--- a/server/test/unittest_storage.py	Tue Jul 26 16:05:36 2011 +0200
+++ b/server/test/unittest_storage.py	Tue Jul 26 16:33:06 2011 +0200
@@ -58,6 +58,7 @@
         self.tempdir = tempfile.mkdtemp()
         bfs_storage = storages.BytesFileSystemStorage(self.tempdir)
         storages.set_attribute_storage(self.repo, 'File', 'data', bfs_storage)
+        storages.set_attribute_storage(self.repo, 'BFSSTestable', 'opt_attr', bfs_storage)
 
     def tearDown(self):
         super(StorageTC, self).tearDown()
@@ -256,6 +257,14 @@
         self.assertEqual(old_path, new_path)
         self.assertEqual(old_data, new_data)
 
+    @tag('update', 'NULL')
+    def test_bfss_update_to_None(self):
+        f = self.session.create_entity('BFSSTestable', opt_attr=Binary('toto'))
+        self.session.commit()
+        self.session.set_pool()
+        f.set_attributes(opt_attr=None)
+        self.session.commit()
+
     @tag('fs_importing', 'update')
     def test_bfss_update_with_fs_importing(self):
         # use self.session to use server-side cache