Merge lp:~brian.curtin/ubuntuone-client/851810-notify-on-volumes into lp:ubuntuone-client

[Brian Curtin]

- Emit a notification when volumes are ready after a server rescan, allowing clients to follow and update their information accordingly (LP: #851810).

After processing volumes during the server rescan, push an event to notify that volumes are ready for clients to consume.

[Brian Curtin (brian.curtin)]

Forget this for now - there are still TODO comments that I forgot to address.

[Brian Curtin (brian.curtin)]

After going over it, I don't believe implementations of the TODO comments were necessary.

[lp:~brian.curtin/ubuntuone-client/851810-notify-on-volumes]

1197. By Natalia Bidart on 2012-02-25

- Use the Qt SSO UI when doing credentials management (LP: #930716).
- Use consistently the U1_DEBUG env var to have DEUBG output.

1198. By dobey on 2012-03-01

Update the messaging indicator integration to launch ubuntuone-installer
Avoid passing time() to the Indicator API which expects GTimeVal now
Update the tests for the changes

1199. By Natalia Bidart on 2012-03-01

- Call CredentialsManagementTool.login when prompt to connect (LP: #944230).

1200. By Natalia Bidart on 2012-03-06

- Made default for MARK logging to be every 15 minutes (LP: #906462).

1201. By Roberto Alsina on 2012-03-06

- Made indicate import optional (LP: #939509).

[Natalia Bidart (nataliabidart)]

Test are not running on linux because:

  File "/home/nessita/canonical/client/review_851810-notify-on-volumes/ubuntuone/platform/linux/dbus_interface.py", line 426, in Shares
    signature='a{ss}')
  File "/usr/lib/python2.7/dist-packages/dbus/decorators.py", line 333, in decorator
    raise ValueError('signal signature is longer than the number of arguments provided')
ValueError: signal signature is longer than the number of arguments provided
make: *** [test] Error 1

This is caused by the following:

73 + @dbus.service.signal(DBUS_IFACE_SHARES_NAME,
74 + signature='a{ss}')
75 + def VolumesReady(self):
76 + """Volumes are ready."""
77 +

VolumesReady should accept a param named 'volumes', like the signature specifies.

Also, in DBus you can't pass an array of objects, so you need to serialize the resulting volumes list. In IRL this means that with this branch we're getting this failure in the logs:

2012-03-07 12:49:39,359 - ubuntuone.SyncDaemon.EQ - ERROR - Error encountered while handling: VM_VOLUMES_READY in <ubuntuone.syncdaemon.interaction_interfaces.SyncdaemonEventListener object at 0xb501ed0>
Traceback (most recent call last):
  File "/home/nessita/canonical/client/review_851810-notify-on-volumes/ubuntuone/syncdaemon/event_queue.py", line 304, in _dispatch
    method(**kwargs)
  File "/home/nessita/canonical/client/review_851810-notify-on-volumes/ubuntuone/logger.py", line 269, in inner
    res = f(*args, **kwargs)
  File "/home/nessita/canonical/client/review_851810-notify-on-volumes/ubuntuone/syncdaemon/interaction_interfaces.py", line 1044, in handle_VM_VOLUMES_READY
    self.interface.shares.VolumesReady(volumes)
  File "/usr/lib/python2.7/dist-packages/dbus/decorators.py", line 314, in emit_signal
    message.append(signature=signature, *args)
TypeError: list indices must be integers, not RootVolume

So, in ubuntuone/syncdaemon/interaction_interfaces.py", line 1044, in handle_VM_VOLUMES_READY, you need to serialize the 'volumes' content. To do so, please use get_share_dict to serialize a the root and shares, and get_udf_dict for folders.

There is another thing to fix: the current volumes list has objects which classes are RooVolume, ShareVolume, and UDFVolume. These classes come from the protocol directly and are "low level", and should not be leaked to other modules. The volume_manager python module has a higher level abstraction for those, see the class Root, Shares and UDF (all 3 inheriting from Volume).

So, the volumes list send in the VM_VOLUMES_READY event should be a list of specializations of volume_manager.Volume. The fix does not ends there :-), as you can see in the

    549 def server_rescan(self):

method, after self._volumes_rescan_cb is done, another call is being to cleanup dead local volumes, so the event VM_VOLUMES_READY should be emitted once that cleanup is completed. Ergo, to solve both of these items (having high-level volume abstractions and a fully updated volumes list), I would suggest removing this line:

    639 events.append(("VM_VOLUMES_READY", dict(volumes=volumes)))

and adding a callback to the...

review: Needs Fixing

[Natalia Bidart (nataliabidart)]

Just tested this change IRL and I'm getting:

  File "/home/nessita/canonical/client/review_851810-notify-on-volumes/ubuntuone/syncdaemon/volume_manager.py", line 562, in server_rescan
    volumes = self.shares + self.udfs
exceptions.TypeError: unsupported operand type(s) for +: 'VMTritcaskShelf' and 'VMTritcaskShelf'

we need to ask verterok how to properly "sum" to volumes list.

[Natalia Bidart (nataliabidart)]

Ok, just chatted with verterok and what we need is to emit the event with this as the volumes param:

volumes = list(self.get_volumes(all_volumes=True))

[lp:~brian.curtin/ubuntuone-client/851810-notify-on-volumes]

1202. By Alejandro J. Cura on 2012-03-08

- The infrastructure for a QtNetwork based process to tunnel syncdaemon traffic thru proxies. (LP: #929207)

1203. By Alejandro J. Cura on 2012-03-09

- A client for the proxy tunnel, to be used by syncdaemon (LP: #929207).

1204. By Alejandro J. Cura on 2012-03-09

- A proxy tunnel process to be started when proxies are enabled (LP: #929207).

1205. By dobey on 2012-03-12

Set the transient hint on the notifications to True

1206. By Alejandro J. Cura on 2012-03-12

- Tunnel storage protocol if proxy enabled in system settings (LP: #929208).

1207. By dobey on 2012-03-13

Fix the typo in the u1sdtool --list-shared help text and man page
Don't install the really old preferences man file we don't need

1208. By Alejandro J. Cura on 2012-03-15

- Use the txweb webclient from sso for webcalls, so they can be proxied too (LP: #929207, LP: #929212).

1209. By Alejandro J. Cura on 2012-03-15

QNetwork must use the proxy built; forward disconnections in the tunnel client.

1210. By dobey on 2012-03-16

Don't attach old useless log files, and clean up the report a little

1211. By Alejandro J. Cura on 2012-03-16

- Use proxy credentials from the keyring (LP: #929207)

1212. By Alejandro J. Cura on 2012-03-18

- Only allow connections that provide the right cookie thru the tunnel (LP: #929207).

1213. By Roberto Alsina on 2012-03-19

Fix tunnel spawning code so that it works on windows.

1214. By Brian Curtin on 2012-03-21

- Loosen the is_root check on Windows to allow Windows XP users, who commonly run with Administrator rights, to start U1 properly. (LP: #930398).

1215. By Brian Curtin on 2012-03-22

merge lp:~brian.curtin/ubuntuone-client/851810-notify-on-volumes

1216. By Brian Curtin on 2012-03-22

match the signature on linux

[Natalia Bidart (nataliabidart)]

I will hijack the branch and propose a new one at: https://code.launchpad.net/~nataliabidart/ubuntuone-client/851810-notify-on-volumes/+merge/99829

review: Needs Resubmitting

=== modified file 'tests/platform/test_external_interface.py'
--- tests/platform/test_external_interface.py	2012-02-03 14:29:52 +0000
+++ tests/platform/test_external_interface.py	2012-03-22 18:12:20 +0000
@@ -31,6 +31,7 @@
 
 STR = 'something'
 STR_STR_DICT = {'foo': 'bar'}
+STR_LST_DICT = {'foo': ['bar', 'baz']}
 
 
 class StatusTests(StatusTestCase):
@@ -294,6 +295,7 @@
         ('ShareSubscribeError', (STR_STR_DICT, STR)),
         ('ShareUnSubscribed', (STR_STR_DICT,)),
         ('ShareUnSubscribeError', (STR_STR_DICT, STR)),
+        ('VolumesReady', (STR_LST_DICT))
     ]
 
     @defer.inlineCallbacks

=== modified file 'tests/syncdaemon/test_interaction_interfaces.py'
--- tests/syncdaemon/test_interaction_interfaces.py	2012-03-01 19:08:12 +0000
+++ tests/syncdaemon/test_interaction_interfaces.py	2012-03-22 18:12:20 +0000
@@ -1750,6 +1750,20 @@
         self.assertEqual(error, error_msg)
 
     @defer.inlineCallbacks
+    def test_handle_VM_VOLUMES_READY(self):
+        """Test the handle_VM_VOLUMES_READY method."""
+        share = self._create_share(accepted=True)
+        udf = self._create_udf()
+        volumes = [share, udf, self.main.vm.root]
+        d = defer.Deferred()
+        self.patch(self.sd_obj.interface.shares, 'VolumesReady', d.callback)
+        self.main.event_q.push('VM_VOLUMES_READY', volumes=volumes)
+
+        info = yield d
+
+        self.assertEqual(volumes, info)
+
+    @defer.inlineCallbacks
     def test_handle_VM_VOLUME_DELETED_folder(self):
         """Test the handle_VM_VOLUME_DELETED method for a folder."""
         udf = self._create_udf()

=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2012-02-09 23:32:10 +0000
+++ tests/syncdaemon/test_vm.py	2012-03-22 18:12:20 +0000
@@ -3104,11 +3104,15 @@
                          vol_rescan_d.callback)  # autosubscribe is False
         server_rescan_d = defer.Deferred()
         self._listen_for('SYS_SERVER_RESCAN_DONE', server_rescan_d.callback)
+        volumes_ready_d = defer.Deferred()
+        self._listen_for('VM_VOLUMES_READY', volumes_ready_d.callback)
         yield self.vm.server_rescan()
         yield server_rescan_d
         events = yield vol_rescan_d
+        vols = yield volumes_ready_d
 
         self.assertEqual({'generation': 1, 'volume_id': ''}, events)
+        self.assertEqual({'volumes' : response}, vols)
 
     @defer.inlineCallbacks
     def test_server_rescan_with_share_autosubscribe(self):

=== modified file 'ubuntuone/platform/linux/dbus_interface.py'
--- ubuntuone/platform/linux/dbus_interface.py	2012-02-18 16:13:04 +0000
+++ ubuntuone/platform/linux/dbus_interface.py	2012-03-22 18:12:20 +0000
@@ -422,6 +422,11 @@
     def ShareDeleteError(self, share_dict, error):
         """An error occurred while deleting a share."""
 
+    @dbus.service.signal(DBUS_IFACE_SHARES_NAME,
+                         signature='a{ss}')
+    def VolumesReady(self, volumes):
+        """Volumes are ready."""
+
     @dbus.service.method(DBUS_IFACE_SHARES_NAME,
                          in_signature='ssss', out_signature='')
     def create_share(self, path, username, name, access_level):

=== modified file 'ubuntuone/platform/tools/windows.py'
--- ubuntuone/platform/tools/windows.py	2011-11-30 14:53:56 +0000
+++ ubuntuone/platform/tools/windows.py	2012-03-22 18:12:20 +0000
@@ -88,6 +88,7 @@
         'ShareUnSubscribed': ('shares', 'on_share_unsubscribed_cb'),
         'ShareUnSubscribeError': ('shares', 'on_share_unsubscribe_error_cb'),
         'StatusChanged': ('status', 'on_status_changed_cb'),
+        'VolumesReady': ('volumes', 'on_volumes_ready_cb'),
     }
 
     # All methods and instance variables that should not be handled by

=== modified file 'ubuntuone/platform/windows/ipc.py'
--- ubuntuone/platform/windows/ipc.py	2012-02-18 16:13:04 +0000
+++ ubuntuone/platform/windows/ipc.py	2012-03-22 18:12:20 +0000
@@ -507,6 +507,7 @@
         'ShareSubscribeError': 'on_share_subscribe_error',
         'ShareUnSubscribed': 'on_share_unsubscribed',
         'ShareUnSubscribeError': 'on_share_unsubscribe_error',
+        'VolumesReady': 'on_volumes_ready',
     }
 
     def get_shares(self):
@@ -616,6 +617,10 @@
     def ShareUnSubscribeError(self, share_info, error):
         """Notify an error while unsubscribing from a share."""
 
+    @signal
+    def VolumesReady(self, volumes):
+        """Volumes are ready."""
+
 
 class Config(IPCExposedObject):
     """The Syncdaemon config/settings interface."""

=== modified file 'ubuntuone/platform/windows/ipc_client.py'
--- ubuntuone/platform/windows/ipc_client.py	2012-02-03 14:29:52 +0000
+++ ubuntuone/platform/windows/ipc_client.py	2012-03-22 18:12:20 +0000
@@ -340,6 +340,7 @@
         'on_share_subscribe_error',
         'on_share_unsubscribed',
         'on_share_unsubscribe_error',
+        'on_volumes_ready',
     ]
 
     @remote
@@ -447,6 +448,10 @@
     def on_share_unsubscribe_error(self, share_id, error):
         """Emit the ShareUnSubscribeError signal."""
 
+    @signal
+    def on_volumes_ready(self, volumes):
+        """Emit VolumesReady signal."""
+
 
 class ConfigClient(RemoteClient):
     """The Syncdaemon config/settings ipc interface. """

=== modified file 'ubuntuone/syncdaemon/event_queue.py'
--- ubuntuone/syncdaemon/event_queue.py	2011-10-14 20:02:23 +0000
+++ ubuntuone/syncdaemon/event_queue.py	2012-03-22 18:12:20 +0000
@@ -160,6 +160,7 @@
     'VM_VOLUME_DELETED': ('volume',),
     'VM_VOLUME_DELETE_ERROR': ('volume_id', 'error'),
     'VM_SHARE_CHANGED': ('share_id',),
+    'VM_VOLUMES_READY': ('volumes',),
 
 }
 

=== modified file 'ubuntuone/syncdaemon/interaction_interfaces.py'
--- ubuntuone/syncdaemon/interaction_interfaces.py	2012-03-19 13:20:36 +0000
+++ ubuntuone/syncdaemon/interaction_interfaces.py	2012-03-22 18:12:20 +0000
@@ -1039,6 +1039,11 @@
         self.interface.shares.ShareChanged(get_share_dict(share))
 
     @log_call(logger.debug)
+    def handle_VM_VOLUMES_READY(self, volumes):
+        """Handle VM_VOLUMES_READY event, emit VolumesReady event."""
+        self.interface.shares.VolumesReady(volumes)
+
+    @log_call(logger.debug)
     def handle_AQ_CHANGE_PUBLIC_ACCESS_OK(self, share_id, node_id,
                                           is_public, public_url):
         """Handle the AQ_CHANGE_PUBLIC_ACCESS_OK event."""

=== modified file 'ubuntuone/syncdaemon/volume_manager.py'
--- ubuntuone/syncdaemon/volume_manager.py	2012-02-09 23:32:10 +0000
+++ ubuntuone/syncdaemon/volume_manager.py	2012-03-22 18:12:20 +0000
@@ -636,6 +636,8 @@
             # update the free_bytes on the volume
             self.update_free_space(volume_id, new_volume.free_bytes)
 
+        events.append(("VM_VOLUMES_READY", dict(volumes=volumes)))
+
         # push the collected events
         for event in events:
             self.m.event_q.push(event[0], **event[1])