[Sugar-devel] [PATCH 3/7] metadatastore: store/change files on disk defensively #2317

[Martin Langhoff]

 - only delete metadata files for keys that are being removed

 - only write files when the data changes

 - write/replace metadata files atomically, to avoid corrupting
   existing data in case of an error

With this patch, we no longer corrupt metadata when trying
to edit/update a ds entry with the system hitting ENOSPC.

Signed-off-by: Martin Langhoff <martin at laptop.org>
---
 src/carquinyol/metadatastore.py |   51 ++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/src/carquinyol/metadatastore.py b/src/carquinyol/metadatastore.py
index 5967017..52cc10f 100644
--- a/src/carquinyol/metadatastore.py
+++ b/src/carquinyol/metadatastore.py
@@ -14,27 +14,46 @@ class MetadataStore(object):
         if not os.path.exists(metadata_path):
             os.makedirs(metadata_path)
         else:
+            received_keys = metadata.keys()
             for key in os.listdir(metadata_path):
-                if key not in _INTERNAL_KEYS:
+                if key not in _INTERNAL_KEYS and key not in received_keys:
                     os.remove(os.path.join(metadata_path, key))
 
         metadata['uid'] = uid
         for key, value in metadata.items():
+            self._set_property(uid, key, value, md_path=metadata_path)
 
-            # Hack to support activities that still pass properties named as
+    def _set_property(self, uid, key, value, md_path=False):
+        if not md_path:
+            md_path = layoutmanager.get_instance().get_metadata_path(uid)
+        # Hack to support activities that still pass properties named as
             # for example title:text.
-            if ':' in key:
-                key = key.split(':', 1)[0]
-
-            f = open(os.path.join(metadata_path, key), 'w')
-            try:
-                if isinstance(value, unicode):
-                    value = value.encode('utf-8')
-                elif not isinstance(value, basestring):
-                    value = str(value)
-                f.write(value)
-            finally:
-                f.close()
+        if ':' in key:
+            key = key.split(':', 1)[0]
+
+        changed = True
+        fpath = os.path.join(md_path, key)
+        tpath = os.path.join(md_path, '.' + key)
+        # FIXME: this codepath handles raw image data
+        # str() is 8-bit clean right now, but
+        # this won't last. We will need more explicit
+        # handling of strings, int/floats vs raw data
+        if isinstance(value, unicode):
+            value = value.encode('utf-8')
+        elif not isinstance(value, basestring):
+            value = str(value)
+
+        # avoid pointless writes; replace atomically
+        if os.path.exists(fpath):
+            stored_val = open(fpath, 'r').read()
+
+            if stored_val == value:
+                changed = False
+        if changed:
+            f = open(tpath, 'w')
+            f.write(value)
+            f.close()
+            os.rename(tpath, fpath)
 
     def retrieve(self, uid, properties=None):
         metadata_path = layoutmanager.get_instance().get_metadata_path(uid)
@@ -55,6 +74,4 @@ class MetadataStore(object):
             return None
 
     def set_property(self, uid, key, value):
-        metadata_path = layoutmanager.get_instance().get_metadata_path(uid)
-        property_path = os.path.join(metadata_path, key)
-        open(property_path, 'w').write(value)
+        self._set_property(uid, key, value)
-- 
1.7.10.4