Merge lp:~verterok/ubuntuone-client/fix-632439 into lp:ubuntuone-client

[Guillermo Gonzalez]

Fix VolumeManager.subscribe_udf method to handle the error cases properly.

Fix a bug in volume manager subscribe_udf method, also improve error handling and propagate it in as a deferred with a failure.

[John O'Brien (jdobrien)]

Looks good, nice catch

review: Approve

[Lucio Torre (lucio.torre)]

simple and nice fix. i wonder if
d = defer.fail(error)
and just one point of exit "return d" would have been better.

review: Approve

[lp:~verterok/ubuntuone-client/fix-632439]

679. By Guillermo Gonzalez on 2010-09-08

merge with trunk

680. By Guillermo Gonzalez on 2010-09-08

make it nicer, remove multiple returns

681. By Guillermo Gonzalez on 2010-09-08

merge with trunk

682. By Guillermo Gonzalez on 2010-09-08

remove print

Unmerged revisions

=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2010-08-04 19:34:03 +0000
+++ tests/syncdaemon/test_vm.py	2010-09-08 18:04:42 +0000
@@ -1192,6 +1192,41 @@
         self.assertTrue(os.path.exists(udf.path))
 
     @defer.inlineCallbacks
+    def test_subscribe_udf_missing_fsm_md(self):
+        """Test VolumeManager.subscribe_udf with a missing node in fsm."""
+        # create and add a UDF
+        suggested_path = "suggested_path"
+        udf, volume = self._create_udf(uuid.uuid4(), 'udf_node_id',
+                                       '~/' + suggested_path, subscribed=False)
+        yield self.vm.add_udf(udf)
+        self.assertFalse(os.path.exists(udf.path))
+        self.assertFalse(self.vm.udfs[udf.id].subscribed)
+        yield self.vm.subscribe_udf(udf.id)
+        yield self.vm.unsubscribe_udf(udf.id)
+        # delete the fsm metadata
+        self.main.fs.delete_metadata(udf.path)
+        # subscribe to it and fail!
+        try:
+            yield self.vm.subscribe_udf(udf.id)
+        except KeyError, e:
+            self.assertIn(udf.path, e.args[0])
+        else:
+            self.fail('Must get a KeyError!')
+
+    @defer.inlineCallbacks
+    def test_subscribe_udf_missing_volume(self):
+        """Test VolumeManager.subscribe_udf with a invalid volume_id."""
+        # create and add a UDF
+        try:
+            r = yield self.vm.subscribe_udf('invalid_udf_id')
+            print r
+        except VolumeDoesNotExist, e:
+            self.assertEquals('DOES_NOT_EXIST', e.args[0])
+            self.assertEquals('invalid_udf_id', e.args[1])
+        else:
+            self.fail('Must get a VolumeDoesNotExist!')
+
+    @defer.inlineCallbacks
     def _test_subscribe_udf_generations(self, udf):
         """Test subscribe_udf with a generation."""
         scratch_d = defer.Deferred()

=== modified file 'ubuntuone/syncdaemon/volume_manager.py'
--- ubuntuone/syncdaemon/volume_manager.py	2010-08-04 19:34:03 +0000
+++ ubuntuone/syncdaemon/volume_manager.py	2010-09-08 18:04:42 +0000
@@ -1093,6 +1093,7 @@
             udf = self.udfs[udf_id]
         except KeyError:
             push_error("DOES_NOT_EXIST")
+            d = defer.fail(VolumeDoesNotExist(udf_id))
         else:
             if not os.path.exists(udf.path):
                 # the udf path isn't there, create it!
@@ -1109,14 +1110,15 @@
                 return result
             try:
                 d = self._scan_udf(udf)
-            except KeyError:
+            except KeyError, e:
                 push_error("METADATA_DOES_NOT_EXIST")
+                d = defer.fail(e)
             else:
                 d.addCallback(subscribe)
                 d.addCallbacks(
                     lambda _: self.m.event_q.push('VM_UDF_SUBSCRIBED', udf),
                     lambda f: push_error(f.getErrorMessage()))
-            return d
+        return d
 
     def _scan_udf(self, udf):
         """Local and server rescan of a UDF."""