Merge lp:~verterok/ubuntuone-client/really-fix-share-changed-handling into lp:ubuntuone-client

[Guillermo Gonzalez]

fix VolumeManager to correctly handle UUID's as share id

[Guillermo Gonzalez (verterok)]

This branch fix VolumeManager to correctly handle UUID's (previously assumed the share_id was always a str)

[Rick McBride (rmcbride)]

Very nice.

review: Approve

[Facundo Batista (facundo)]

Looks fine.

review: Approve

=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2009-08-11 14:33:43 +0000
+++ tests/syncdaemon/test_vm.py	2009-08-12 19:49:30 +0000
@@ -120,7 +120,8 @@
 
     def test_handle_AQ_SHARES_LIST(self):
         """ test the handling of the AQ_SHARE_LIST event. """
-        share_response = ShareResponse.from_params('share_id', 'to_me',
+        share_id = uuid.uuid4()
+        share_response = ShareResponse.from_params(share_id, 'to_me',
                                                    'fake_share_uuid',
                                                    'fake_share', 'username',
                                                    'visible_username', 'yes',
@@ -132,14 +133,17 @@
         self.vm.handle_AQ_SHARES_LIST(response)
         self.assertEquals(2, len(self.vm.shares)) # the new shares and root
         # check that the share is in the shares dict
-        self.assertTrue('share_id' in self.vm.shares)
-        share = self.vm.shares['share_id']
+        self.assertTrue(str(share_id) in self.vm.shares)
+        share = self.vm.shares[str(share_id)]
         self.assertEquals('fake_share', share.name)
         self.assertEquals('fake_share_uuid', share.subtree)
 
+
+
     def test_handle_SV_SHARE_CHANGED(self):
         """ test the handling of the AQ_SHARE_LIST event. """
-        share_holder = NotifyShareHolder.from_params('share_id', None,
+        share_id = uuid.uuid4()
+        share_holder = NotifyShareHolder.from_params(share_id, None,
                                                      'fake_share',
                                                      'test_username',
                                                      'visible_name', 'Modify')
@@ -147,11 +151,11 @@
         self.vm.on_server_root('root_uuid')
         # create a share
         share_path = os.path.join(self.shares_dir, share_holder.share_name)
-        share = Share(share_path, share_id=share_holder.share_id,
+        share = Share(share_path, share_id=str(share_holder.share_id),
                       access_level='View')
         self.vm.add_share(share)
         self.vm.handle_SV_SHARE_CHANGED(message='changed', info=share_holder)
-        self.assertEquals('Modify', self.vm.shares['share_id'].access_level)
+        self.assertEquals('Modify', self.vm.shares[str(share_id)].access_level)
         self.vm.handle_SV_SHARE_CHANGED('deleted', share_holder)
         self.assertNotIn('share_id', self.vm.shares)
 
@@ -167,12 +171,14 @@
 
     def test_handle_AQ_SHARES_LIST_shared(self):
         """test the handling of the AQ_SHARE_LIST event, with a shared dir."""
-        share_response = ShareResponse.from_params('share_id', 'to_me',
+        share_id = uuid.uuid4()
+        share_response = ShareResponse.from_params(share_id, 'to_me',
                                                    'fake_share_uuid',
                                                    'fake_share', 'username',
                                                    'visible_username', 'yes',
                                                    'View')
-        shared_response = ShareResponse.from_params('shared_id', 'from_me',
+        shared_id = uuid.uuid4()
+        shared_response = ShareResponse.from_params(shared_id, 'from_me',
                                                    'shared_uuid',
                                                    'fake_shared', 'myname',
                                                    'my_visible_name', 'yes',
@@ -187,7 +193,7 @@
         self.vm.handle_AQ_SHARES_LIST(response)
         self.assertEquals(2, len(self.vm.shares)) # the new shares and root
         self.assertEquals(1, len(self.vm.shared)) # the new shares and root
-        shared = self.vm.shared['shared_id']
+        shared = self.vm.shared[str(shared_id)]
         self.assertEquals('fake_shared', shared.name)
         # check that the uuid is stored in fs
         mdobj = self.main.fs.get_by_path(shared.path)

=== modified file 'ubuntuone/syncdaemon/volume_manager.py'
--- ubuntuone/syncdaemon/volume_manager.py	2009-08-11 14:33:43 +0000
+++ ubuntuone/syncdaemon/volume_manager.py	2009-08-12 20:13:15 +0000
@@ -246,7 +246,7 @@
     def handle_SV_SHARE_CHANGED(self, message, info):
         """ handle SV_SHARE_CHANGED event """
         if message == 'changed':
-            if info.share_id not in self.shares:
+            if str(info.share_id) not in self.shares:
                 self.log.debug("New share notification, share_id: %s",
                          info.share_id)
                 dir_name = self._build_dirname(info.share_name,
@@ -259,7 +259,7 @@
                 self.share_changed(info)
         elif message == 'deleted':
             self.log.debug('share deleted! %s', info.share_id)
-            self.share_deleted(info.share_id)
+            self.share_deleted(str(info.share_id))
 
     def handle_AQ_CREATE_SHARE_OK(self, share_id, marker):
         """ handle AQ_CREATE_SHARE_OK event. """
@@ -334,14 +334,15 @@
 
     def share_changed(self, share_holder):
         """ process the share changed event """
-        share = self.shares.get(share_holder.share_id, None)
+        # the holder id is a uuid, use the str
+        share = self.shares.get(str(share_holder.share_id), None)
         if share is None:
             # we don't have this share, ignore it
             self.log.warning("Got a share changed notification (%r), "
                              "but don't have the share", share_holder.share_id)
         else:
             share.access_level = share_holder.access_level
-            self.shares[share_holder.share_id] = share
+            self.shares[share.id] = share
 
     def _create_share_dir(self, share):
         """ Creates the share root dir, and set the permissions. """