Merge lp:~verterok/ubuntuone-client/fix-645519-stable-1-4 into lp:ubuntuone-client/stable-1-4

[Guillermo Gonzalez]

Fix delete_share DBus method to actually delete shares

This branch fix delete_share DBus method to actually delete shares (from_me), and mainting support for deleting share volumes (to me).

[lp:~verterok/ubuntuone-client/fix-645519-stable-1-4]

718. By Guillermo Gonzalez on 2010-09-23

fix duplicated docstring

[Natalia Bidart (nataliabidart)]

review: Approve

[Facundo Batista (facundo)]

Looks ok

review: Approve

=== modified file 'contrib/testing/testcase.py'
--- contrib/testing/testcase.py	2010-09-07 16:02:04 +0000
+++ contrib/testing/testcase.py	2010-09-23 18:31:45 +0000
@@ -145,6 +145,8 @@
         """Send the event."""
         self.eq.push('AQ_ANSWER_SHARE_OK', share_id, answer)
 
+    def delete_share(self, share_id):
+        """Stub."""
 
     def disconnect(self, *a, **k):
         """Stub implementation."""

=== modified file 'tests/syncdaemon/test_action_queue.py'
--- tests/syncdaemon/test_action_queue.py	2010-09-14 16:30:15 +0000
+++ tests/syncdaemon/test_action_queue.py	2010-09-23 18:31:45 +0000
@@ -51,7 +51,7 @@
     ActionQueue, ActionQueueCommand, ChangePublicAccess, CreateUDF,
     DeleteVolume, Download, ListVolumes,
     NoisyRequestQueue, RequestQueue, UploadProgressWrapper, Upload,
-    CreateShare, GetPublicFiles, GetDelta, GetDeltaFromScratch,
+    CreateShare, DeleteShare, GetPublicFiles, GetDelta, GetDeltaFromScratch,
     TRANSFER_PROGRESS_THRESHOLD, Unlink, Move, MakeFile, MakeDir,
     DeltaList, ZipQueue, DeferredMap, UniqueRequestQueue
 )
@@ -64,6 +64,7 @@
 VOLUME = uuid.UUID('12345678-1234-1234-1234-123456789abc')
 NODE = uuid.UUID('FEDCBA98-7654-3211-2345-6789ABCDEF12')
 USER = u'Dude'
+SHARE = uuid.uuid4()
 
 NO_OP = lambda *args, **kwargs: None
 
@@ -2681,6 +2682,54 @@
         self.assertEqual(len(self.queue), 0)
 
 
+class DeleteShareTestCase(ConnectedBaseTestCase):
+    """Test for DeleteShare ActionQueueCommand."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Init."""
+        yield super(DeleteShareTestCase, self).setUp()
+        request_queue = RequestQueue(name='fu', action_queue=self.action_queue)
+        self.command = DeleteShare(request_queue, SHARE)
+        vm = self.action_queue.main.vm
+        self.patch(vm, 'handle_AQ_DELETE_SHARE_OK', NO_OP)
+        self.patch(vm, 'handle_AQ_DELETE_SHARE_ERROR', NO_OP)
+
+    def test_run_returns_a_deferred(self):
+        """Test a deferred is returned."""
+        res = self.command._run()
+        self.assertTrue(isinstance(res, defer.Deferred), 'deferred returned')
+
+    def test_run_calls_protocol(self):
+        """Test protocol's delete_volume is called."""
+        self.called = False
+        def check(share_id):
+            """Take control over client's feature."""
+            self.called = True
+            self.assertEqual(SHARE, share_id)
+        self.patch(self.command.action_queue.client, 'delete_share', check)
+        self.command._run()
+        self.assertTrue(self.called, "command wasn't called")
+
+    def test_handle_success_push_event(self):
+        """Test AQ_DELETE_SHARE_OK is pushed on success."""
+        request = client.DeleteShare(self.action_queue.client, SHARE)
+        res = self.command.handle_success(success=request)
+        events = [('AQ_DELETE_SHARE_OK', (), {'share_id': SHARE})]
+        self.assertEqual(events, self.command.action_queue.event_queue.events)
+        self.assertEqual(request, res)
+
+    def test_handle_failure_push_event(self):
+        """Test AQ_DELETE_SHARE_ERROR is pushed on failure."""
+        msg = 'Something went wrong'
+        failure = Failure(DefaultException(msg))
+        res = self.command.handle_failure(failure=failure)
+        events = [('AQ_DELETE_SHARE_ERROR', (),
+                  {'share_id': SHARE, 'error': msg})]
+        self.assertEqual(events, self.command.action_queue.event_queue.events)
+        self.assertTrue(res is None)
+
+
 class SimpleAQTestCase(BasicTestCase):
     """Simple tests for AQ API."""
 

=== modified file 'tests/syncdaemon/test_dbus.py'
--- tests/syncdaemon/test_dbus.py	2010-09-13 16:55:35 +0000
+++ tests/syncdaemon/test_dbus.py	2010-09-23 18:31:45 +0000
@@ -2885,6 +2885,31 @@
                            error_handler=self.error_handler)
         return d
 
+    def test_delete_share_from_me(self):
+        """Test for Shares.delete_share with share from_me."""
+        share_path = os.path.join(self.main.shares_dir, 'share')
+        share = Shared(path=share_path, volume_id='share_id',
+                      node_id='node_id', accepted=True)
+        self.main.vm.add_shared(share)
+        d = defer.Deferred()
+        def delete_share(volume_id):
+            """Fake delete_volume."""
+            self.main.event_q.push("AQ_DELETE_SHARE_OK", share_id=volume_id)
+        self.patch(self.main.action_q, 'delete_share', delete_share)
+        def deleted_handler(info):
+            """ShareDeleted handler."""
+            self.assertRaises(KeyError,
+                              self.main.vm.shared.__getitem__, info['volume_id'])
+            d.callback(True)
+        match = self.bus.add_signal_receiver(deleted_handler,
+                                             signal_name='ShareDeleted')
+        self.signal_receivers.add(match)
+        client = DBusClient(self.bus, '/shares', DBUS_IFACE_SHARES_NAME)
+        client.call_method('delete_share', share.volume_id,
+                           reply_handler=lambda _: None,
+                           error_handler=self.error_handler)
+        return d
+
     def test_delete_share_error_signal(self):
         """Test for Shares.delete_share with an error."""
         share_path = os.path.join(self.main.shares_dir, 'share')
@@ -2912,6 +2937,50 @@
                            error_handler=self.error_handler)
         return d
 
+    def test_delete_share_from_me_error(self):
+        """Test failure of Shares.delete_share with a share from_me."""
+        share_path = os.path.join(self.main.shares_dir, 'share')
+        share = Shared(path=share_path, volume_id='share_id',
+                      node_id='node_id', accepted=True)
+        self.main.vm.add_shared(share)
+        d = defer.Deferred()
+        # patch delete_share to fail
+        def delete_share(share_id):
+            """Fake delete_share."""
+            self.main.event_q.push("AQ_DELETE_SHARE_ERROR",
+                                   share_id=share_id, error="I'm broken")
+        self.patch(self.main.action_q, 'delete_share', delete_share)
+        def deleted_error_handler(info, error):
+            """FolderDeleteError handler."""
+            self.assertEquals(info['volume_id'], share.volume_id)
+            self.assertEquals(error, "I'm broken")
+            d.callback(True)
+        match = self.bus.add_signal_receiver(deleted_error_handler,
+                                             signal_name='ShareDeleteError')
+        self.signal_receivers.add(match)
+        client = DBusClient(self.bus, '/shares', DBUS_IFACE_SHARES_NAME)
+        client.call_method('delete_share', share.volume_id,
+                           reply_handler=lambda *args: None,
+                           error_handler=self.error_handler)
+        return d
+
+    def test_delete_share_from_me_doesnotexist(self):
+        """Test failure of Shares.delete_share with a share from_me."""
+        d = defer.Deferred()
+        # patch delete_share to fail
+        def deleted_error_handler(info, error):
+            """FolderDeleteError handler."""
+            self.assertEquals(info['volume_id'], 'missing_share_id')
+            self.assertEquals(error, "DOES_NOT_EXIST")
+            d.callback(True)
+        match = self.bus.add_signal_receiver(deleted_error_handler,
+                                             signal_name='ShareDeleteError')
+        self.signal_receivers.add(match)
+        client = DBusClient(self.bus, '/shares', DBUS_IFACE_SHARES_NAME)
+        client.call_method('delete_share', 'missing_share_id',
+                           reply_handler=lambda *args: None,
+                           error_handler=self.error_handler)
+        return d
 
 def test_suite():
     # pylint: disable-msg=C0111

=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2010-09-16 20:38:05 +0000
+++ tests/syncdaemon/test_vm.py	2010-09-23 18:31:45 +0000
@@ -412,6 +412,63 @@
         share = self.vm.shared[str(expected_share_id)]
         self.assertEquals(str(expected_node_id), share.node_id)
 
+    @defer.inlineCallbacks
+    def test_delete_shared(self):
+        """Test VolumeManager.delete_share."""
+        path = os.path.join(self.vm.root.path, 'shared_path')
+        self.main.fs.create(path, "")
+        self.main.fs.set_node_id(path, 'node_id')
+        share = Shared(path=path, volume_id='share_id', node_id="node_id")
+        self.vm.add_shared(share)
+
+        def fake_delete_share(share_id):
+            """Fake delete_share."""
+            self.assertEqual(share_id, share.volume_id)
+            self.main.event_q.push('AQ_DELETE_SHARE_OK', share_id)
+
+        self.patch(self.main.action_q, 'delete_share', fake_delete_share)
+        d = defer.Deferred()
+        self._listen_for('VM_SHARE_DELETED', d.callback, 1, collect=True)
+        self.vm.delete_share(share.volume_id)
+        events = yield d
+        event = events[0]
+        self.assertEqual(event[0], (share,))
+
+    @defer.inlineCallbacks
+    def test_delete_shared_error(self):
+        """Test VolumeManager.delete_share."""
+        path = os.path.join(self.vm.root.path, 'shared_path')
+        self.main.fs.create(path, "")
+        self.main.fs.set_node_id(path, 'node_id')
+        share = Shared(path=path, volume_id='share_id', node_id="node_id")
+        self.vm.add_shared(share)
+
+        def fake_delete_share(share_id):
+            """Fake delete_share that always fails."""
+            self.main.event_q.push('AQ_DELETE_SHARE_ERROR',
+                                   share_id, 'a fake error')
+
+        self.patch(self.main.action_q, 'delete_share', fake_delete_share)
+        d = defer.Deferred()
+        self._listen_for('VM_SHARE_DELETE_ERROR', d.callback, 1, collect=True)
+        self.vm.delete_share(share.volume_id)
+        events = yield d
+        event = events[0]
+        self.assertEqual(event[0], (share.volume_id, 'a fake error'))
+
+    @defer.inlineCallbacks
+    def test_delete_shared_error_missing_share(self):
+        """Test VolumeManager.delete_share with a non-existent share."""
+        path = os.path.join(self.vm.root.path, 'shared_path')
+        self.main.fs.create(path, "")
+        self.main.fs.set_node_id(path, 'node_id')
+        d = defer.Deferred()
+        self._listen_for('VM_SHARE_DELETE_ERROR', d.callback, 1, collect=True)
+        self.vm.delete_share('fake_share_id')
+        events = yield d
+        event = events[0]
+        self.assertEqual(event[0], ('fake_share_id', 'DOES_NOT_EXIST'))
+
     def test_accept_share(self):
         """ Test the accept_share method. """
         d = defer.Deferred()
@@ -680,6 +737,37 @@
         self.assertTrue(os.path.exists(share.path),
                         'share path missing on disk!')
 
+    @defer.inlineCallbacks
+    def test_handle_AQ_DELETE_SHARE_OK(self):
+        """Test for handle_AQ_DELETE_SHARE_OK."""
+        path = os.path.join(self.vm.root.path, 'shared_path')
+        self.main.fs.create(path, "")
+        self.main.fs.set_node_id(path, 'node_id')
+        share = Shared(path=path, volume_id='share_id', node_id="node_id")
+        self.vm.add_shared(share)
+        d = defer.Deferred()
+        self._listen_for('VM_SHARE_DELETED', d.callback, 1, collect=True)
+        self.main.event_q.push('AQ_DELETE_SHARE_OK', 'share_id')
+        events = yield d
+        event = events[0]
+        self.assertEqual(event[0], (share,))
+
+    @defer.inlineCallbacks
+    def test_handle_AQ_DELETE_SHARE_ERROR(self):
+        """Test for handle_AQ_DELETE_SHARE_ERROR."""
+        path = os.path.join(self.vm.root.path, 'shared_path')
+        self.main.fs.create(path, "")
+        self.main.fs.set_node_id(path, 'node_id')
+        share = Shared(path=path, volume_id='share_id', node_id="node_id")
+        self.vm.add_shared(share)
+        d = defer.Deferred()
+        self._listen_for('VM_SHARE_DELETE_ERROR', d.callback, 1, collect=True)
+        self.main.event_q.push('AQ_DELETE_SHARE_ERROR',
+                               'share_id', 'a fake error')
+        events = yield d
+        event = events[0]
+        self.assertEqual(event[0], (share.volume_id, 'a fake error'))
+
 
 class VolumeManagerUnicodeTests(BaseVolumeManagerTests):
     """Tests for Volume Manager unicode capabilities."""
@@ -1008,8 +1096,8 @@
         """Test the handling of the AQ_LIST_VOLUMES event."""
         share_id = uuid.uuid4()
         share_volume = volumes.ShareVolume(share_id, 'fake_share_uuid', None,
-                                           10, 'to_me', u'ñoño'.encode("utf8"),
-                                           'username', 'visible_username',
+                                           10, 'to_me', u'ñoño',
+                                           'username', u'visible_username',
                                            True, 'View')
         udf_id = uuid.uuid4()
         udf_volume = volumes.UDFVolume(udf_id, 'udf_uuid', None, 10, u'~/ñoño')
@@ -1028,7 +1116,7 @@
         self.assertIn(str(share_id), self.vm.shares)
         self.assertIn(str(udf_id), self.vm.udfs)
         share = self.vm.shares[str(share_id)]
-        self.assertEqual(u'ñoño'.encode("utf8"), share.name)
+        self.assertEqual(u'ñoño', share.name)
         self.assertEqual('fake_share_uuid', share.node_id)
         udf = self.vm.udfs[str(udf_id)]
         self.assertEqual('udf_uuid', udf.node_id)
@@ -1140,8 +1228,8 @@
         yield self.vm.add_udf(udf)
         share_id = uuid.uuid4()
         share_volume = volumes.ShareVolume(share_id, 'fake_share_uuid', None,
-                                           10, 'to_me', u'ñoño'.encode("utf8"),
-                                           'username', 'visible_username',
+                                           10, 'to_me', u'ñoño',
+                                           'username', u'visible_username',
                                            True, 'View')
         dir_name = self.vm._build_share_path(share_volume.share_name,
                                           share_volume.other_visible_name)
@@ -1604,8 +1692,8 @@
         udf_volume = volumes.UDFVolume(udf_id, 'udf_uuid', None, 10, u'~/ñoño')
         share_id = uuid.uuid4()
         share_volume = volumes.ShareVolume(share_id, 'fake_share_uuid', None,
-                                           10, 'to_me', u'ñoño'.encode("utf8"),
-                                           'username', 'visible_username',
+                                           10, 'to_me', u'ñoño',
+                                           'username', u'visible_username',
                                            True, 'View')
         # initialize the the root
         self.vm._got_root('root_uuid')
@@ -1656,8 +1744,8 @@
         """Test for handle_SV_VOLUME_DELETED."""
         share_id = uuid.uuid4()
         share_volume = volumes.ShareVolume(share_id, 'fake_share_uuid', None,
-                                           10, 'to_me', u'ñoño'.encode("utf8"),
-                                           'username', 'visible_username',
+                                           10, 'to_me', u'ñoño',
+                                           'username', u'visible_username',
                                            True, 'View')
         dir_name = self.vm._build_share_path(share_volume.share_name,
                                           share_volume.other_visible_name)
@@ -1665,8 +1753,7 @@
         share = Share.from_share_volume(share_volume, share_path)
         # create a UDF
         udf_id = uuid.uuid4()
-        udf, udf_volume = self._create_udf(udf_id, 'udf_uuid',
-                                           u'~/ñoño'.encode("utf8"))
+        udf, udf_volume = self._create_udf(udf_id, 'udf_uuid', u'~/ñoño')
         yield self.vm.add_udf(udf)
         self.vm.add_share(share)
         # initialize the the root

=== modified file 'ubuntuone/syncdaemon/action_queue.py'
--- ubuntuone/syncdaemon/action_queue.py	2010-09-14 16:30:15 +0000
+++ ubuntuone/syncdaemon/action_queue.py	2010-09-23 18:31:45 +0000
@@ -1016,6 +1016,10 @@
         return CreateShare(self.meta_queue, node_id, share_to, name,
                            access_level, marker).queue()
 
+    def delete_share(self, share_id):
+        """Delete a offered share."""
+        return DeleteShare(self.meta_queue, share_id).queue()
+
     def create_udf(self, path, name, marker):
         """Create a User Defined Folder.
 
@@ -1703,6 +1707,32 @@
                                            error=failure.getErrorMessage())
 
 
+class DeleteShare(ActionQueueCommand):
+    """Delete a offered Share."""
+
+    __slots__ = ('share_id',)
+
+    def __init__(self, request_queue, share_id):
+        super(DeleteShare, self).__init__(request_queue)
+        self.share_id = share_id
+
+    def _run(self):
+        """Do the actual running."""
+        return self.action_queue.client.delete_share(self.share_id)
+
+    def handle_success(self, success):
+        """It worked! Push the event."""
+        self.action_queue.event_queue.push('AQ_DELETE_SHARE_OK',
+                                           share_id=self.share_id)
+        return success
+
+    def handle_failure(self, failure):
+        """It didn't work. Push the event."""
+        self.action_queue.event_queue.push('AQ_DELETE_SHARE_ERROR',
+                                           share_id=self.share_id,
+                                           error=failure.getErrorMessage())
+
+
 class CreateUDF(ActionQueueCommand):
     """Create a new User Defined Folder."""
 

=== modified file 'ubuntuone/syncdaemon/dbus_interface.py'
--- ubuntuone/syncdaemon/dbus_interface.py	2010-09-16 16:54:48 +0000
+++ ubuntuone/syncdaemon/dbus_interface.py	2010-09-23 18:31:45 +0000
@@ -788,6 +788,16 @@
         self.handle_default('VM_SHARE_CREATED', share_id)
         self.dbus_iface.shares.emit_new_share(share_id)
 
+    def handle_VM_SHARE_DELETED(self, share):
+        """Handle VM_SHARE_DELETED event, emit NewShare event."""
+        self.handle_default('VM_SHARE_DELETED', getattr(share, 'id', share))
+        self.dbus_iface.shares.emit_share_changed('deleted', share)
+
+    def handle_VM_SHARE_DELETE_ERROR(self, share_id, error):
+        """Handle VM_DELETE_SHARE_ERROR event, emit ShareCreateError signal."""
+        self.handle_default('VM_SHARE_DELETE_ERROR', share_id, error)
+        self.dbus_iface.shares.ShareDeleteError(dict(volume_id=share_id), error)
+
     def handle_VM_VOLUME_DELETED(self, volume):
         """Handle VM_VOLUME_DELETED event.
 
@@ -804,7 +814,7 @@
                      "volume_id=%r as it's not a share or UDF", volume.id)
 
     def handle_VM_VOLUME_DELETE_ERROR(self, volume_id, error):
-        """Handle VM_SHARE_DELETED event, emit ShareDeleted event."""
+        """Handle VM_VOLUME_DELETE_ERROR event, emit ShareDeleted event."""
         self.handle_default('VM_VOLUME_DELETE_ERROR', volume_id, error)
         try:
             volume = self.dbus_iface.volume_manager.get_volume(volume_id)
@@ -1131,21 +1141,21 @@
                                      str(share_id)))
 
     @dbus.service.method(DBUS_IFACE_SHARES_NAME,
-                         in_signature='s', out_signature='',
-                         async_callbacks=('reply_handler', 'error_handler'))
-    def delete_share(self, share_id, reply_handler=None, error_handler=None):
-        """Delete a Share."""
+                         in_signature='s', out_signature='')
+    def delete_share(self, share_id):
+        """Delete a Share, both kinds: "to me" and "from me"."""
         logger.debug('delete_share: %r', share_id)
         try:
-            self.vm.delete_volume(str(share_id))
-            reply_handler()
-        except VolumeDoesNotExist, e:
-            self.ShareDeleteError({'volume_id':share_id}, str(e))
+            try:
+                self.vm.delete_volume(str(share_id))
+            except VolumeDoesNotExist:
+                # isn't a volume! it might be a "share from me (a.k.a shared)"
+                self.vm.delete_share(str(share_id))
         except Exception, e:
-            error_handler(e)
             logger.exception('Error while deleting share: %r', share_id)
             self.ShareDeleteError({'volume_id':share_id}, str(e))
-
+            # propagate the error
+            raise
 
     @dbus.service.signal(DBUS_IFACE_SHARES_NAME,
                          signature='a{ss}')

=== modified file 'ubuntuone/syncdaemon/event_queue.py'
--- ubuntuone/syncdaemon/event_queue.py	2010-08-31 14:19:53 +0000
+++ ubuntuone/syncdaemon/event_queue.py	2010-09-23 18:31:45 +0000
@@ -69,6 +69,8 @@
     'AQ_LIST_SHARES_ERROR': ('error',),
     'AQ_CREATE_SHARE_OK': ('share_id', 'marker'),
     'AQ_CREATE_SHARE_ERROR': ('marker', 'error'),
+    'AQ_DELETE_SHARE_OK': ('share_id',),
+    'AQ_DELETE_SHARE_ERROR': ('share_id', 'error'),
     'AQ_QUERY_ERROR': ('item', 'error'),
     'AQ_ANSWER_SHARE_OK': ('share_id', 'answer'),
     'AQ_ANSWER_SHARE_ERROR': ('share_id', 'answer', 'error'),
@@ -145,6 +147,8 @@
     'VM_UDF_CREATED': ('udf',),
     'VM_UDF_CREATE_ERROR': ('path', 'error'),
     'VM_SHARE_CREATED': ('share_id',),
+    'VM_SHARE_DELETED': ('udf',),
+    'VM_SHARE_DELETE_ERROR': ('path', 'error'),
     'VM_VOLUME_DELETED': ('volume',),
     'VM_VOLUME_DELETE_ERROR': ('volume_id', 'error'),
     'VM_SHARE_CHANGED': ('share_id',),

=== modified file 'ubuntuone/syncdaemon/volume_manager.py'
--- ubuntuone/syncdaemon/volume_manager.py	2010-09-16 20:38:05 +0000
+++ ubuntuone/syncdaemon/volume_manager.py	2010-09-23 18:31:45 +0000
@@ -717,6 +717,22 @@
         if marker in self.marker_share_map:
             del self.marker_share_map[marker]
 
+    def handle_AQ_DELETE_SHARE_OK(self, share_id):
+        """Handle AQ_DELETE_SHARE_OK event."""
+        share = self.shared.get(share_id)
+        if share is not None:
+            del self.shared[share_id]
+            self.m.event_q.push('VM_SHARE_DELETED', share)
+        else:
+            # delete ok, of something we don't have.
+            self.log.warning("AQ_DELETE_SHARE_OK of a non-existing shared "
+                             "dir: %s", share_id)
+
+    def handle_AQ_DELETE_SHARE_ERROR(self, share_id, error):
+        """Gandle AQ_DELETE_SHARE_ERROR event."""
+        self.log.error("Error while deleting offered share: %s", error)
+        self.m.event_q.push('VM_SHARE_DELETE_ERROR', share_id, error)
+
     def handle_SV_SHARE_ANSWERED(self, share_id, answer):
         """ handle SV_SHARE_ANSWERED event. """
         share = self.shared.get(share_id, None)
@@ -908,6 +924,15 @@
         self.m.action_q.create_share(node_id, username, name,
                                      access_level, marker)
 
+    def delete_share(self, share_id):
+        """Reuqest the deletion of an offered share."""
+        share = self.shared.get(share_id)
+        if share is None:
+            # if the share isn't there, push the error!
+            self.m.event_q.push('VM_SHARE_DELETE_ERROR', share_id, "DOES_NOT_EXIST")
+        else:
+            self.m.action_q.delete_share(share_id)
+
     def add_shared(self, share):
         """ Add a share with direction == from_me """
         self.log.info('New shared subtree: id: %s - path: %r',