Merge lp:~diegosarmentero/ubuntuone-client/ipcmenu into lp:ubuntuone-client

[Diego Sarmentero]

- Adding ipc support to share the menu data (LP: #1032659).

[Roberto Alsina (ralsina)]

review: Approve

[Brian Curtin (brian.curtin)]

review: Approve

[Alejandro J. Cura (alecu)]

Please make constants out of the keys for the data dict ('recent-transfers' and 'uploading'), so that the strings are only defined once and use those constants through the branch. This makes it faster to catch mistakes.

Please provide detailed information on the keys, and very detailed info too on the dbus types of each of the possible values, of the dbus dictionary of variants returned by the method exposed on dbus. This should be done as a multiline docstring of the exported method.

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-client/ipcmenu]

1296. By Diego Sarmentero on 2012-08-10

Adding more complete doctrings and making some strings constants.

[Diego Sarmentero (diegosarmentero)]

> Please make constants out of the keys for the data dict ('recent-transfers'
> and 'uploading'), so that the strings are only defined once and use those
> constants through the branch. This makes it faster to catch mistakes.
>
> Please provide detailed information on the keys, and very detailed info too on
> the dbus types of each of the possible values, of the dbus dictionary of
> variants returned by the method exposed on dbus. This should be done as a
> multiline docstring of the exported method.

Fixed!

[Alejandro J. Cura (alecu)]

19 + return {RECENT_TRANSFERS: (), UPLOADING: ()}

123 + {UPLOADING: expected, RECENT_TRANSFERS: ()})

133 + {UPLOADING: expected, RECENT_TRANSFERS: ()})

All of this should be empty lists, not tuples.
Both in Python and DBus, lists are used to signify collections of homogeneous items, while tuples or dbus.Structs are used for collections of heterogeneous items, so it makes better sense to use lists there.

---

162 +
163 + This method return the following structure:
164 + {
165 + 'recent-transfers': (str),
166 + 'uploading': (str, int, int)
167 + }

This notation does not imply that each of those are lists, and also the doc is not explicit about the contents of each field. Please replace with something like this:

"""
This method returns a dictionary, with the following keys and values:

Key: 'recent-transfers'
Value: a list of strings, each being the name of a file that was recently transferred.

Key: 'uploading'
Value: a list of tuples, with each tuple having the following items:
 * str: the name of a file that's currently being uploaded
 * int: ???
 * int: ???
"""

---

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-client/ipcmenu]

1297. By Diego Sarmentero on 2012-08-13

fixing docstrings

1298. By Diego Sarmentero on 2012-08-13

fixing tests

[Alejandro J. Cura (alecu)]

+1

review: Approve

=== modified file 'contrib/testing/testcase.py'
--- contrib/testing/testcase.py	2012-06-22 09:59:14 +0000
+++ contrib/testing/testcase.py	2012-08-13 15:04:21 +0000
@@ -57,6 +57,8 @@
     main,
     local_rescan,
     tritcask,
+    RECENT_TRANSFERS,
+    UPLOADING,
 )
 from ubuntuone.syncdaemon import logger
 from ubuntuone import platform
@@ -208,6 +210,10 @@
 
     show_all_notifications = True
 
+    def menu_data(self):
+        """Fake menu_data."""
+        return {RECENT_TRANSFERS: [], UPLOADING: []}
+
 
 class FakeMain(main.Main):
     """ A fake Main class to setup the tests """

=== modified file 'tests/platform/ipc/test_external_interface.py'
--- tests/platform/ipc/test_external_interface.py	2012-08-03 14:56:05 +0000
+++ tests/platform/ipc/test_external_interface.py	2012-08-13 15:04:21 +0000
@@ -40,6 +40,10 @@
     StatusTestCase,
     SyncDaemonTestCase,
 )
+from ubuntuone.syncdaemon import (
+    RECENT_TRANSFERS,
+    UPLOADING,
+)
 
 STR = 'something'
 STR_STR_DICT = {'foo': 'bar'}
@@ -132,6 +136,16 @@
         self.assert_remote_method('waiting_metadata',
             in_signature=None, out_signature='a(sa{ss})')
 
+    @defer.inlineCallbacks
+    def test_sync_menu(self):
+        """Test sync_menu."""
+        result = {RECENT_TRANSFERS: [], UPLOADING: []}
+        method = 'sync_menu'
+        yield self.assert_method_called(self.service.status,
+                                        method, result)
+        self.assert_remote_method(method,
+            in_signature=None, out_signature='a{sv}')
+
 
 class EventsTests(EventsTestCase):
     """Basic tests for the Events exposed object."""

=== modified file 'tests/platform/test_tools.py'
--- tests/platform/test_tools.py	2012-05-30 15:35:49 +0000
+++ tests/platform/test_tools.py	2012-08-13 15:04:21 +0000
@@ -36,12 +36,15 @@
 from ubuntuone.devtools.handlers import MementoHandler
 
 from contrib.testing.testcase import FakeCommand, skipIfOS
+
 from ubuntuone.syncdaemon import (
     action_queue,
     event_queue,
     interaction_interfaces,
     states,
     volume_manager,
+    RECENT_TRANSFERS,
+    UPLOADING,
 )
 from ubuntuone.platform import tools
 from tests.platform import IPCTestCase
@@ -243,6 +246,13 @@
         self.assertEqual('share_id', result['volume_id'])
         self.assertEqual(False, self.main.vm.shares['share_id'].accepted)
 
+    @defer.inlineCallbacks
+    def test_sync_menu(self):
+        """Test accept_share method."""
+        result = yield self.tool.sync_menu()
+        self.assertIn(RECENT_TRANSFERS, result)
+        self.assertIn(UPLOADING, result)
+
 
 class TestWaitForSignals(TestToolsBase):
     """Test case for the wait_for_signals method from SyncDaemonTool."""

=== modified file 'tests/status/test_aggregator.py'
--- tests/status/test_aggregator.py	2012-08-06 14:25:15 +0000
+++ tests/status/test_aggregator.py	2012-08-13 15:04:21 +0000
@@ -42,7 +42,11 @@
 from ubuntuone.status import aggregator
 from ubuntuone.status.notification import AbstractNotification
 from ubuntuone.status.messaging import AbstractMessaging
-from ubuntuone.syncdaemon import status_listener
+from ubuntuone.syncdaemon import (
+    status_listener,
+    RECENT_TRANSFERS,
+    UPLOADING,
+)
 from ubuntuone.syncdaemon.volume_manager import Share, UDF, Root
 
 FILENAME = 'example.txt'
@@ -813,11 +817,12 @@
         self.listener.handle_SYS_QUEUE_ADDED(fake_command)
         self.listener.handle_SYS_QUEUE_REMOVED(fake_command)
         transfers = self.status_frontend.recent_transfers()
-        expected = ('path1', 'path2', 'path3')
+        expected = ['path1', 'path2', 'path3']
         self.assertEqual(transfers, expected)
 
         menu_data = self.listener.menu_data()
-        self.assertEqual(menu_data, ([], expected))
+        self.assertEqual(menu_data,
+            {UPLOADING: [], RECENT_TRANSFERS: expected})
 
     def test_file_uploading(self):
         """Check that it returns a list with the path, size, and progress."""
@@ -827,7 +832,8 @@
         expected = [('testfile.txt', 0, 0)]
         self.assertEqual(uploading, expected)
         menu_data = self.listener.menu_data()
-        self.assertEqual(menu_data, (expected, ()))
+        self.assertEqual(menu_data,
+            {UPLOADING: expected, RECENT_TRANSFERS: []})
 
         fc.size = 1000
         fc.n_bytes_written = 200
@@ -840,7 +846,8 @@
         self.assertEqual(uploading, expected)
 
         menu_data = self.listener.menu_data()
-        self.assertEqual(menu_data, (expected, ()))
+        self.assertEqual(menu_data,
+            {UPLOADING: expected, RECENT_TRANSFERS: []})
 
     def test_menu_data_full_response(self):
         """Check that listener.menu_data returns both uploading and recent."""
@@ -854,9 +861,11 @@
         self.status_frontend.upload_started(fc)
         uploading = self.status_frontend.files_uploading()
         transfers = self.status_frontend.recent_transfers()
-        expected = ([('testfile.txt', 1000, 200)], ('path1',))
+        expected = {UPLOADING: [('testfile.txt', 1000, 200)],
+            RECENT_TRANSFERS: ['path1']}
 
-        self.assertEqual((uploading, transfers), expected)
+        self.assertEqual(
+            {UPLOADING: uploading, RECENT_TRANSFERS: transfers}, expected)
 
     def test_file_published(self):
         """A file published event is processed."""

=== modified file 'ubuntuone/platform/ipc/ipc_client.py'
--- ubuntuone/platform/ipc/ipc_client.py	2012-05-22 14:28:56 +0000
+++ ubuntuone/platform/ipc/ipc_client.py	2012-08-13 15:04:21 +0000
@@ -197,6 +197,22 @@
     def current_uploads(self):
         """Return a list of files with a upload in progress."""
 
+    @remote
+    def sync_menu(self):
+        """
+        This method returns a dictionary, with the following keys and values:
+
+        Key: 'recent-transfers'
+        Value: a list of strings (paths), each being the name of a file that
+               was recently transferred.
+
+        Key: 'uploading'
+        Value: a list of tuples, with each tuple having the following items:
+         * str: the path of a file that's currently being uploaded
+         * int: size of the file
+         * int: bytes written
+        """
+
     @signal
     def on_content_queue_changed(self):
         """Emit ContentQueueChanged."""

=== modified file 'ubuntuone/platform/ipc/linux.py'
--- ubuntuone/platform/ipc/linux.py	2012-05-22 14:07:55 +0000
+++ ubuntuone/platform/ipc/linux.py	2012-08-13 15:04:21 +0000
@@ -38,6 +38,10 @@
 from xml.etree import ElementTree
 
 from ubuntuone.platform.launcher import UbuntuOneLauncher
+from ubuntuone.syncdaemon import (
+    RECENT_TRANSFERS,
+    UPLOADING,
+)
 
 # Disable the "Invalid Name" check here, as we have lots of DBus style names
 # pylint: disable-msg=C0103
@@ -173,6 +177,35 @@
         warnings.warn('Use "waiting" method instead.', DeprecationWarning)
         return self.service.status.waiting_content()
 
+    @dbus.service.method(DBUS_IFACE_STATUS_NAME, out_signature='a{sv}')
+    def sync_menu(self):
+        """
+        This method returns a dictionary, with the following keys and values:
+
+        Key: 'recent-transfers'
+        Value: a list of strings (paths), each being the name of a file that
+               was recently transferred.
+
+        Key: 'uploading'
+        Value: a list of tuples, with each tuple having the following items:
+         * str: the path of a file that's currently being uploaded
+         * int: size of the file
+         * int: bytes written
+        """
+        data = self.service.status.sync_menu()
+        uploading = data[UPLOADING]
+        transfers = data[RECENT_TRANSFERS]
+        upload_data = dbus.Array(signature="(sii)")
+        transfer_data = dbus.Array(signature="s")
+        for up in uploading:
+            upload_data.append(dbus.Struct(up, signature="sii"))
+        for transfer in transfers:
+            transfer_data.append(transfer)
+        result = dbus.Dictionary(signature="sv")
+        result[UPLOADING] = upload_data
+        result[RECENT_TRANSFERS] = transfer_data
+        return result
+
     @dbus.service.signal(DBUS_IFACE_STATUS_NAME)
     def DownloadStarted(self, path):
         """Fire a signal to notify that a download has started."""

=== modified file 'ubuntuone/platform/ipc/perspective_broker.py'
--- ubuntuone/platform/ipc/perspective_broker.py	2012-07-13 16:06:27 +0000
+++ ubuntuone/platform/ipc/perspective_broker.py	2012-08-13 15:04:21 +0000
@@ -231,6 +231,7 @@
         'waiting',
         'waiting_metadata',
         'waiting_content',
+        'sync_menu',
     ]
 
     signal_mapping = {
@@ -291,6 +292,10 @@
         warnings.warn('Use "waiting" method instead.', DeprecationWarning)
         return self.service.status.waiting_content()
 
+    def sync_menu(self):
+        """Return the info necessary to construct the menu."""
+        return self.service.status.sync_menu()
+
     @signal
     def DownloadStarted(self, path):
         """Fire a signal to notify that a download has started."""

=== modified file 'ubuntuone/platform/tools/__init__.py'
--- ubuntuone/platform/tools/__init__.py	2012-05-30 15:35:49 +0000
+++ ubuntuone/platform/tools/__init__.py	2012-08-13 15:04:21 +0000
@@ -326,6 +326,13 @@
 
     @defer.inlineCallbacks
     @log_call(logger.debug)
+    def sync_menu(self):
+        """Return a deferred that will be fired with the sync menu data."""
+        results = yield self.proxy.call_method('status', 'sync_menu')
+        defer.returnValue(results)
+
+    @defer.inlineCallbacks
+    @log_call(logger.debug)
     def accept_share(self, share_id):
         """Accept the share with id: share_id."""
         d = self.wait_for_signals(signal_ok='ShareAnswerResponse',

=== modified file 'ubuntuone/status/aggregator.py'
--- ubuntuone/status/aggregator.py	2012-08-06 14:25:15 +0000
+++ ubuntuone/status/aggregator.py	2012-08-13 15:04:21 +0000
@@ -810,7 +810,7 @@
 
     def recent_transfers(self):
         """Return a tuple with the recent transfers paths."""
-        return tuple(self.aggregator.recent_transfers)
+        return list(self.aggregator.recent_transfers)
 
     def files_uploading(self):
         """Return a list with the files being uploading."""

=== modified file 'ubuntuone/syncdaemon/__init__.py'
--- ubuntuone/syncdaemon/__init__.py	2012-04-09 20:07:05 +0000
+++ ubuntuone/syncdaemon/__init__.py	2012-08-13 15:04:21 +0000
@@ -36,3 +36,8 @@
                            "volumes",
                            "generations",
                           ])
+
+
+#Sync Menu data constants
+RECENT_TRANSFERS = 'recent-transfers'
+UPLOADING = 'uploading'

=== modified file 'ubuntuone/syncdaemon/interaction_interfaces.py'
--- ubuntuone/syncdaemon/interaction_interfaces.py	2012-08-08 18:36:17 +0000
+++ ubuntuone/syncdaemon/interaction_interfaces.py	2012-08-13 15:04:21 +0000
@@ -302,6 +302,10 @@
                 waiting_content.append(data)
         return waiting_content
 
+    def sync_menu(self):
+        """Return the info necessary to construct the menu."""
+        return self.main.status_listener.menu_data()
+
 
 class SyncdaemonFileSystem(SyncdaemonObject):
     """An interface to the FileSystem Manager."""

=== modified file 'ubuntuone/syncdaemon/status_listener.py'
--- ubuntuone/syncdaemon/status_listener.py	2012-08-03 14:56:05 +0000
+++ ubuntuone/syncdaemon/status_listener.py	2012-08-13 15:04:21 +0000
@@ -31,7 +31,12 @@
 """Listener for event queue that updates the UI to show syncdaemon status."""
 
 from ubuntuone.status.aggregator import StatusFrontend
-from ubuntuone.syncdaemon import action_queue, config
+from ubuntuone.syncdaemon import (
+    action_queue,
+    config,
+    RECENT_TRANSFERS,
+    UPLOADING,
+)
 from ubuntuone.syncdaemon.interaction_interfaces import (
     get_share_dict, get_udf_dict)
 from ubuntuone.syncdaemon.volume_manager import UDF, Root
@@ -68,9 +73,10 @@
 
     def menu_data(self):
         """Return the info necessary to construct the sync menu."""
+        uploading = self.status_frontend.files_uploading()
         transfers = self.status_frontend.recent_transfers()
-        uploading = self.status_frontend.files_uploading()
-        return uploading, transfers
+        data = {RECENT_TRANSFERS: transfers, UPLOADING: uploading}
+        return data
 
     def get_show_all_notifications(self):
         """Get the value of show_all_notifications."""