[server,bfss] fix storage migration when Bytes attributes is None (closes #13519541)
authorAlexandre Richardson <alexandre.richardson@logilab.fr>
Tue, 07 Jun 2016 10:05:30 +0200
changeset 11273 c655e19cbc35
parent 11272 53fbd5644bff
child 11276 6eeb7abda47a
[server,bfss] fix storage migration when Bytes attributes is None (closes #13519541) When a Bytes attributes is None before the BFSS storage migration, the migration crashes. A Bytes attribute can be None, when the attribute is not required and there is no default value. In this case, the attributes should remained None after migration and no file is stored on the disk.
cubicweb/server/migractions.py
cubicweb/server/sources/storages.py
cubicweb/server/test/data-migractions/schema.py
cubicweb/server/test/unittest_migractions.py
--- a/cubicweb/server/migractions.py	Wed May 25 09:51:51 2016 +0200
+++ b/cubicweb/server/migractions.py	Tue Jun 07 10:05:30 2016 +0200
@@ -1369,7 +1369,7 @@
             getattr(entity, attribute)
             storage.migrate_entity(entity, attribute)
             # remove from entity cache to avoid memory exhaustion
-            del entity.cw_attr_cache[attribute]
+            entity.cw_attr_cache.pop(attribute, None)
             pb.update()
         print()
         source.set_storage(etype, attribute, storage)
--- a/cubicweb/server/sources/storages.py	Wed May 25 09:51:51 2016 +0200
+++ b/cubicweb/server/sources/storages.py	Tue Jun 07 10:05:30 2016 +0200
@@ -157,12 +157,13 @@
             entity._cw_dont_cache_attribute(attr, repo_side=True)
         else:
             binary = entity.cw_edited.pop(attr)
-            fd, fpath = self.new_fs_path(entity, attr)
-            # bytes storage used to store file's path
-            binary_obj = Binary(fpath if PY2 else fpath.encode('utf-8'))
-            entity.cw_edited.edited_attribute(attr, binary_obj)
-            self._writecontent(fd, binary)
-            AddFileOp.get_instance(entity._cw).add_data(fpath)
+            if binary is not None:
+                fd, fpath = self.new_fs_path(entity, attr)
+                # bytes storage used to store file's path
+                binary_obj = Binary(fpath if PY2 else fpath.encode('utf-8'))
+                entity.cw_edited.edited_attribute(attr, binary_obj)
+                self._writecontent(fd, binary)
+                AddFileOp.get_instance(entity._cw).add_data(fpath)
         return binary
 
     def entity_updated(self, entity, attr):
@@ -259,13 +260,14 @@
     def migrate_entity(self, entity, attribute):
         """migrate an entity attribute to the storage"""
         entity.cw_edited = EditedEntity(entity, **entity.cw_attr_cache)
-        self.entity_added(entity, attribute)
-        cnx = entity._cw
-        source = cnx.repo.system_source
-        attrs = source.preprocess_entity(entity)
-        sql = source.sqlgen.update('cw_' + entity.cw_etype, attrs,
-                                   ['cw_eid'])
-        source.doexec(cnx, sql, attrs)
+        binary = self.entity_added(entity, attribute)
+        if binary is not None:
+            cnx = entity._cw
+            source = cnx.repo.system_source
+            attrs = source.preprocess_entity(entity)
+            sql = source.sqlgen.update('cw_' + entity.cw_etype, attrs,
+                                       ['cw_eid'])
+            source.doexec(cnx, sql, attrs)
         entity.cw_edited = None
 
 
--- a/cubicweb/server/test/data-migractions/schema.py	Wed May 25 09:51:51 2016 +0200
+++ b/cubicweb/server/test/data-migractions/schema.py	Tue Jun 07 10:05:30 2016 +0200
@@ -150,6 +150,7 @@
         })
     description = String()
     firstname = String(fulltextindexed=True, maxsize=64)
+    photo = Bytes()
 
     concerne = SubjectRelation('Affaire')
     connait = SubjectRelation('Personne')
--- a/cubicweb/server/test/unittest_migractions.py	Wed May 25 09:51:51 2016 +0200
+++ b/cubicweb/server/test/unittest_migractions.py	Tue Jun 07 10:05:30 2016 +0200
@@ -20,17 +20,20 @@
 from datetime import date
 import os, os.path as osp
 from contextlib import contextmanager
+import tempfile
 
-from logilab.common.testlib import unittest_main, Tags, tag
+from logilab.common.testlib import unittest_main, Tags, tag, with_tempdir
 from logilab.common import tempattr
 
 from yams.constraints import UniqueConstraint
 
-from cubicweb import ConfigurationError, ValidationError, ExecutionError
+from cubicweb import (ConfigurationError, ValidationError,
+                      ExecutionError, Binary)
 from cubicweb.devtools import startpgcluster, stoppgcluster
 from cubicweb.devtools.testlib import CubicWebTC
 from cubicweb.server.sqlutils import SQL_PREFIX
 from cubicweb.server.migractions import ServerMigrationHelper
+from cubicweb.server.sources import storages
 
 import cubicweb.devtools
 
@@ -488,7 +491,7 @@
                 'X ordernum O')]
             expected = [u'nom', u'prenom', u'sexe', u'promo', u'ass', u'adel', u'titre',
                         u'web', u'tel', u'fax', u'datenaiss', u'test', u'tzdatenaiss',
-                        u'description', u'firstname',
+                        u'description', u'firstname', u'photo',
                         u'creation_date', u'cwuri', u'modification_date']
             self.assertEqual(expected, rinorder)
 
@@ -755,6 +758,29 @@
             mh.drop_relation_definition('Note', 'ecrit_par', 'CWUser')
             self.assertFalse(mh.sqlexec('SELECT * FROM cw_Note WHERE cw_ecrit_par IS NOT NULL'))
 
+    @with_tempdir
+    def test_storage_changed(self):
+        with self.mh() as (cnx, mh):
+            john = mh.cmd_create_entity('Personne', nom=u'john',
+                                        photo=Binary(b'something'))
+            bill = mh.cmd_create_entity('Personne', nom=u'bill')
+            mh.commit()
+            bfs_storage = storages.BytesFileSystemStorage(tempfile.tempdir)
+            storages.set_attribute_storage(self.repo, 'Personne', 'photo', bfs_storage)
+            mh.cmd_storage_changed('Personne', 'photo')
+            bob = mh.cmd_create_entity('Personne', nom=u'bob')
+            bffss_dir_content = os.listdir(tempfile.tempdir)
+            self.assertEqual(len(bffss_dir_content), 1)
+            john.cw_clear_all_caches()
+            self.assertEqual(john.photo.getvalue(),
+                             osp.join(tempfile.tempdir, bffss_dir_content[0]))
+            bob.cw_clear_all_caches()
+            self.assertIsNone(bob.photo)
+            bill.cw_clear_all_caches()
+            self.assertIsNone(bill.photo)
+            storages.unset_attribute_storage(self.repo, 'Personne', 'photo')
+
+
 class MigrationCommandsComputedTC(MigrationTC):
     """ Unit tests for computed relations and attributes
     """