Merge lp:~facundo/magicicada-gui/show-md into lp:magicicada-gui

[Facundo Batista]

Get and deliver metadata for a given path

[Natalia Bidart (nataliabidart)]

Docstring """Processes...""" needs fixing to """Process""". Same for "Gets" and "Returns".

review: Needs Fixing

[Natalia Bidart (nataliabidart)]

I was just thinking... what if the "code" for syncdaemon.get_metadata is the path per se? I think is key enough to guarantee that the metadata result is the one that the GUI/user is waiting for.

I just realized this when implementing the GUI part. Let me know what you think.

[Natalia Bidart (nataliabidart)]

2 more things are needed:

(a) Could you please return a python dictionary instead a dbus dictionary? I'd want to keep dbus out the GTK layer.

(b) If the path doesn't exist, we'd need a special value, not a string saying "Not a valid path". This way we can ease the future translations, and we can detect this special case in a proper way.

[Facundo Batista (facundo)]

After talking through IRC, what will do is:

- Will use "path" itself as the "code".

- The constant used in dbusiface.py in case of no path will be NOT_SYNCHED_PATH

- Will return a Py dict, instead a str

[lp:~facundo/magicicada-gui/show-md]

44. By Facundo Batista on 2010-06-14

Some docstring fixes and small changes.

[Facundo Batista (facundo)]

I just make it to return the DBus types to the GUI.

Naty does not want it, but I thought duck typing should work... even thinking that, I tried to make tests to check that the types were converted, but...

>>> isinstance(dbus.String(u'a'), unicode)
True

and

>>> isinstance(dbus.Dictionary({dbus.String('a'): dbus.String(3)}), dict)
True

So, actually, it would be too tough and too fake to assert that, and it should work rightaway.

[Natalia Bidart (nataliabidart)]

Nice!

review: Approve

=== modified file 'magicicada/dbusiface.py'
--- magicicada/dbusiface.py	2010-06-05 15:57:34 +0000
+++ magicicada/dbusiface.py	2010-06-14 18:14:25 +0000
@@ -50,11 +50,19 @@
                 "(Move)\(share_id=(.*?), node_id=(.*?), old_parent_id=(.*?), "
                 "new_parent_id=(.*?), new_name=(.*?)\)")
 
+# DBus exceptions store the type inside, as a string :|
+DBUSERR_NOREPLY = 'org.freedesktop.DBus.Error.NoReply'
+DBUSERR_NAMENOOWNER = 'org.freedesktop.DBus.Error.NameHasNoOwner'
+DBUSERR_PYKEYERROR = 'org.freedesktop.DBus.Python.KeyError'
+
+# some constants
+NOT_SYNCHED_PATH = "Not a valid path!"
+
 
 def _is_retry_exception(err):
     """Check if the exception is a retry one."""
     if isinstance(err, dbus.exceptions.DBusException):
-        if err.get_dbus_name() == 'org.freedesktop.DBus.Error.NoReply':
+        if err.get_dbus_name() == DBUSERR_NOREPLY:
             return True
     return False
 
@@ -323,8 +331,7 @@
         try:
             self._bus.get_name_owner('com.ubuntuone.SyncDaemon')
         except dbus.exceptions.DBusException, err:
-            if err.get_dbus_name() != \
-                                'org.freedesktop.DBus.Error.NameHasNoOwner':
+            if err.get_dbus_name() != DBUSERR_NAMENOOWNER:
                 raise
             started = False
         else:
@@ -381,3 +388,23 @@
         d = self.sync_daemon_tool.list_shared()
         d.addCallback(process)
         return d
+
+    @retryable
+    def get_metadata(self, path):
+        """Return the raw metadata."""
+        logger.info("Getting metadata for %r", path)
+
+        def fix_failure(failure):
+            """Get the failure and return a nice message."""
+            if failure.check(dbus.exceptions.DBusException):
+                if failure.value.get_dbus_name() == DBUSERR_PYKEYERROR:
+                    return NOT_SYNCHED_PATH
+            return failure
+
+        def process(metadata):
+            logger.debug("Got metadata for path %r: %r", path, metadata)
+            return dict(metadata)
+
+        d = self.sync_daemon_tool.get_metadata(path)
+        d.addCallbacks(process, fix_failure)
+        return d

=== modified file 'magicicada/syncdaemon.py'
--- magicicada/syncdaemon.py	2010-06-11 20:09:11 +0000
+++ magicicada/syncdaemon.py	2010-06-14 18:14:25 +0000
@@ -104,6 +104,7 @@
         self.on_folders_changed_callback = NO_OP
         self.on_shares_to_me_changed_callback = NO_OP
         self.on_shares_to_others_changed_callback = NO_OP
+        self.on_metadata_ready_callback = None  # mandatory
 
         # mq needs to be polled to know progress
         self._mqcaller = None
@@ -282,3 +283,11 @@
         """Tell the SyncDaemon that the user wants it to disconnect."""
         logger.info("Telling u1.SD to disconnect")
         self.dbus.disconnect()
+
+    def get_metadata(self, path):
+        """Get the metadata for given path."""
+        if self.on_metadata_ready_callback is None:
+            raise ValueError("Missing the mandatory cback for get_metadata.")
+
+        d = self.dbus.get_metadata(path)
+        d.addCallback(lambda resp: self.on_metadata_ready_callback(path, resp))

=== modified file 'magicicada/tests/test_dbusiface.py'
--- magicicada/tests/test_dbusiface.py	2010-06-03 03:58:02 +0000
+++ magicicada/tests/test_dbusiface.py	2010-06-14 18:14:25 +0000
@@ -43,7 +43,7 @@
         del self._callbacks[(dbus_interface, signal_name)]
 
     def get_name_owner(self, name):
-        """Fakes the response of the method."""
+        """Fake the response of the method."""
         assert name == 'com.ubuntuone.SyncDaemon'
         if isinstance(self.fake_name_owner, str):
             return self.fake_name_owner
@@ -69,7 +69,10 @@
                     return defer.Deferred()
                 methname, response = self._fake_response
                 assert methname == name
-                return response
+                if isinstance(response, Exception):
+                    return defer.fail(response)
+                else:
+                    return defer.succeed(response)
             return f
 
 
@@ -104,9 +107,8 @@
         return called_args
 
     def fake_sdt_response(self, method_name, response):
-        """Fakes SDT answer in deferred mode."""
-        self.dbus.sync_daemon_tool._fake_response = (method_name,
-                                                     defer.succeed(response))
+        """Fake SDT answer in deferred mode."""
+        self.dbus.sync_daemon_tool._fake_response = (method_name, response)
 
 class TestSignalHooking(SafeTests):
     """Signal hooking tests.
@@ -188,7 +190,7 @@
 
 
 class TestDataProcessingStatus(SafeTests):
-    """Processes Status before sending it to SyncDaemon."""
+    """Process Status before sending it to SyncDaemon."""
 
     @defer.inlineCallbacks
     def test_get_status(self):
@@ -224,7 +226,7 @@
 
 
 class TestDataProcessingNameOwner(SafeTests):
-    """Processes Name Owner data before sending it to SyncDaemon."""
+    """Process Name Owner data before sending it to SyncDaemon."""
 
     def test_name_owner_changed_no_syncdaemon(self):
         """Test name owner changed callback."""
@@ -245,7 +247,7 @@
 
 
 class TestDataProcessingCQ(SafeTests):
-    """Processes CQ data before sending it to SyncDaemon."""
+    """Process CQ data before sending it to SyncDaemon."""
 
     @defer.inlineCallbacks
     def test_nodata(self):
@@ -288,7 +290,7 @@
 
 
 class TestDataProcessingMQ(SafeTests):
-    """Processes MQ data before sending it to SyncDaemon."""
+    """Process MQ data before sending it to SyncDaemon."""
 
     @defer.inlineCallbacks
     def test_nodata(self):
@@ -463,7 +465,7 @@
 
 
 class TestDataProcessingFolders(SafeTests):
-    """Processes Folders data before sending it to SyncDaemon."""
+    """Process Folders data before sending it to SyncDaemon."""
 
     @defer.inlineCallbacks
     def test_nodata(self):
@@ -521,9 +523,29 @@
         self.get_msd_called("on_sd_folders_changed")
 
 
+class TestDataProcessingMetadata(SafeTests):
+    """Process Metadata data before sending it to SyncDaemon."""
+
+    @defer.inlineCallbacks
+    def test_info_ok(self):
+        """Test get metadata and see response."""
+        md = dbus.Dictionary({'a': 3, 'c': 4}, signature=dbus.Signature('ss'))
+        self.fake_sdt_response('get_metadata', md)
+        rcv = yield self.dbus.get_metadata('path')
+        self.assertEqual(rcv, dict(a=3, c=4))
+
+    @defer.inlineCallbacks
+    def test_info_bad(self):
+        """Test get metadata and get the error."""
+        exc = dbus.exceptions.DBusException(
+                                name='org.freedesktop.DBus.Python.KeyError')
+        self.fake_sdt_response('get_metadata', exc)
+        rcv = yield self.dbus.get_metadata('not a real path')
+        self.assertEqual(rcv, dbusiface.NOT_SYNCHED_PATH)
+
 
 class TestDataProcessingShares(SafeTests):
-    """Processes Shares data before sending it to SyncDaemon."""
+    """Process Shares data before sending it to SyncDaemon."""
 
     @defer.inlineCallbacks
     def test_sharestome_nodata(self):
@@ -720,6 +742,11 @@
         self.dbus.get_folders()
         self.assertTrue(self.handler.check_inf("Getting folders"))
 
+    def test_get_metadata(self):
+        """Test call to metadata."""
+        self.dbus.get_metadata('path')
+        self.assertTrue(self.handler.check_inf("Getting metadata for u'path'"))
+
     def test_get_shares_to_me(self):
         """Test call to shares to me."""
         self.dbus.get_shares_to_me()
@@ -847,6 +874,15 @@
         self.assertTrue(self.handler.check_dbg("    Folders data: %r" % d))
 
     @defer.inlineCallbacks
+    def test_metadata_processing(self):
+        """Test get metadata."""
+        d = dict(lot_of_data="I don't care")
+        self.fake_sdt_response('get_metadata', d)
+        yield self.dbus.get_metadata('path')
+        self.assertTrue(self.handler.check_dbg(
+                        "Got metadata for path u'path': %r" % d))
+
+    @defer.inlineCallbacks
     def test_sharestome_processing(self):
         """Test get shares to me with one."""
         d = dict(accepted=u'True', access_level=u'View', free_bytes=u'123456',
@@ -877,7 +913,7 @@
     """Test the retry decorator."""
 
     class Helper(object):
-        """Fails some times, finally succeeds."""
+        """Fail some times, finally succeed."""
         def __init__(self, limit, excep=None):
             self.cant = 0
             self.limit = limit
@@ -915,7 +951,7 @@
         self.assertTrue(dbusiface._is_retry_exception(err))
 
     def get_decorated_func(self, func):
-        """Executes the test calling the received function."""
+        """Execute the test calling the received function."""
 
         @dbusiface.retryable
         def f():

=== modified file 'magicicada/tests/test_syncdaemon.py'
--- magicicada/tests/test_syncdaemon.py	2010-06-11 20:09:11 +0000
+++ magicicada/tests/test_syncdaemon.py	2010-06-14 18:14:25 +0000
@@ -133,7 +133,7 @@
 
     @defer.inlineCallbacks
     def test_initial_value(self):
-        """Fills the status info initially."""
+        """Fill the status info initially."""
         called = []
         def fake():
             """Fake method."""
@@ -167,7 +167,7 @@
         return deferred
 
     def test_status_changed_affects_cuurent_status(self):
-        """Makes changes to see how status are reflected."""
+        """Make changes to see how status are reflected."""
         # one set of values
         self.sd.on_sd_status_changed('name1', 'description1', False, True,
                                      False, 'queues1', 'connection1')
@@ -261,7 +261,7 @@
 
     @defer.inlineCallbacks
     def test_initial_value(self):
-        """Fills the content queue info initially."""
+        """Fill the content queue info initially."""
         called = []
         self.sd.dbus.get_content_queue = lambda: called.append(True)
         yield self.sd._get_initial_data()
@@ -341,7 +341,7 @@
 
     @defer.inlineCallbacks
     def test_initial_value(self):
-        """Fills the meta queue info initially."""
+        """Fill the meta queue info initially."""
         called = []
         def f():
             """Helper function."""
@@ -777,11 +777,38 @@
         self.assertTrue(self.hdlr.check_inf("SD Name Owner changed: True"))
 
 
+class MetadataTests(BaseTest):
+    """Get Metadata info."""
+
+    def test_get_metadata_no_callback_set(self):
+        """It's mandatory to set the callback for this response."""
+        self.assertRaises(ValueError, self.sd.get_metadata, 'path')
+
+    def test_get_metadata_ok(self):
+        """Get the metadata for given path."""
+        called = []
+        self.sd.dbus.get_metadata = lambda p: defer.succeed('foo')
+        self.sd.on_metadata_ready_callback = lambda *a: called.extend(a)
+        self.sd.get_metadata('path')
+        self.assertEqual(called, ['path', 'foo'])
+
+    def test_get_metadata_double(self):
+        """Get the metadata twice."""
+        called = []
+        fake_md = {'path1': 'foo', 'path2': 'bar'}
+        self.sd.dbus.get_metadata = lambda p: defer.succeed(fake_md[p])
+        self.sd.on_metadata_ready_callback = lambda *a: called.append(a)
+        self.sd.get_metadata('path1')
+        self.sd.get_metadata('path2')
+        self.assertEqual(called[0], ('path1', 'foo'))
+        self.assertEqual(called[1], ('path2', 'bar'))
+
+
 class FoldersTests(BaseTest):
     """Folders checking."""
 
     def test_foldercreated_callback(self):
-        """Gets the new data after the folders changed."""
+        """Get the new data after the folders changed."""
         # set the callback
         called = []
         self.sd.dbus.get_folders = lambda: called.append(True)
@@ -794,7 +821,7 @@
 
     @defer.inlineCallbacks
     def test_initial_value(self):
-        """Fills the folder info initially."""
+        """Fill the folder info initially."""
         called = []
         self.sd.dbus.get_folders = lambda: called.append(True)
         yield self.sd._get_initial_data()
@@ -817,7 +844,7 @@
     """Shares checking."""
 
     def test_shares_changed_callback(self):
-        """Gets the new data after the shares changed."""
+        """Get the new data after the shares changed."""
         # set the callback
         called = []
         self.sd.dbus.get_shares_to_me = lambda: called.append(True)
@@ -831,7 +858,7 @@
 
     @defer.inlineCallbacks
     def test_initial_value(self):
-        """Fills the folder info initially."""
+        """Fill the folder info initially."""
         called = []
         self.sd.dbus.get_shares_to_me = lambda: called.append(True)
         self.sd.dbus.get_shares_to_others = lambda: called.append(True)