Merge lp:~mvo/software-center/dataprovider-desktop-dependency-property into lp:software-center

[Michael Vogt]

Small branch for the unity guys

[Michael Vogt (mvo)]

Setting back to WIP until I get a +1 from the unity people

[Michael Vogt (mvo)]

According to pstolowski it crashes with:

(process:24210): unity-applications-daemon-WARNING **: daemon.vala:1263: Failed to get package details for 'application://keepassx.desktop': GDBus.Error:org.freedesktop.DBus.Python.TypeError: Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/dbus/service.py", line 751, in _message_cb
    _method_reply_return(connection, message, method_name, signature, *retval)
  File "/usr/lib/python2.7/dist-packages/dbus/service.py", line 254, in _method_reply_return
    reply.append(signature=signature, *retval)
TypeError: Don't know which D-Bus type to use to encode type "set"

so definitely a issue with the test not catching this.

[Michael Vogt (mvo)]

Setting to needs-review based on:
"""
From: Pawel Stolowski
Hi Michael,

Your branch now works fine, thanks a lot for your work!
"""

[Michael Nelson (michael.nelson)]

06:47 < mvo> I wonder if I could get a quick review for https://code.launchpad.net/~mvo/software-center/dataprovider-desktop-dependency-property/+merge/124462 to merge/upload it in before the b2 freeze (which will probably start around noon today)
06:47 * noodles looks
06:49 < noodles> mvo: why do you need lines 17-19 when you've got lines 23?
06:49 < noodles> mvo: ah, nm.
06:49 < noodles> I see (I'd reversed the removed/added)
06:52 < noodles> mvo: looks great. I'm assuming you didn't mean to leave 183-184 in the diff? But whatever - see what you think.
06:52 * noodles adds +1.
06:53 < noodles> mvo: also, it'd be pretty easy to update the two tests that you've already touched there to actually test values when there's a set, or empty list or empty dict? Just a thought.

review: Approve

[lp:~mvo/software-center/dataprovider-desktop-dependency-property]

3179. By Michael Vogt on 2012-09-18

improve tests to actually cover AppDetails.as_dbus_property_dict() explicitely

3180. By Michael Vogt on 2012-09-18

trivial docstring updates

3181. By Michael Vogt on 2012-09-18

add real dbus bus test

=== modified file 'softwarecenter/db/application.py'
--- softwarecenter/db/application.py	2012-09-14 09:19:09 +0000
+++ softwarecenter/db/application.py	2012-09-18 08:18:20 +0000
@@ -261,7 +261,7 @@
         raise ValueError("pkg '%s' has not archive_suite '%s'" % (
                 pkg, archive_suite))
 
-    def as_property_dict(self):
+    def as_dbus_property_dict(self):
         """ return all properties as a dict with name/value """
         import inspect
         properties = {}
@@ -270,12 +270,19 @@
         for name, value in inspect.getmembers(self):
             if name.startswith("_") or callable(value) or name in NOT_EXPOSE:
                 continue
+            # dbus does not like sets
+            if isinstance(value, set):
+                value = list(value)
             # make sure that the type can be encoded
             if not type(value) in [
-                list, dict, tuple, set, basestring, int, float, bool, None]:
+                list, dict, tuple, basestring, int, float, bool, None]:
                 value = "%s" % value
+            # dbus does not like empty dicts/lists
+            if isinstance(value, dict) or isinstance(value, list):
+                if len(value) == 0:
+                    value = ""
             # and set it
-            properties[name] = value or ""
+            properties[name] = value
         return properties
 
     @property
@@ -408,6 +415,15 @@
         return self._history.get_installed_date(self.pkgname)
 
     @property
+    def is_desktop_dependency(self):
+        if self._pkg:
+            depends = self._cache.get_packages_removed_on_remove(
+                self._pkg)
+            if "ubuntu-desktop" in depends:
+                return True
+        return False
+
+    @property
     def purchase_date(self):
         if self._doc:
             return self._doc.get_value(XapianValues.PURCHASED_DATE)
 
=== modified file 'softwarecenter/db/dataprovider.py'
--- softwarecenter/db/dataprovider.py	2012-09-14 07:12:33 +0000
+++ softwarecenter/db/dataprovider.py	2012-09-18 08:18:20 +0000
@@ -35,20 +35,24 @@
 from softwarecenter.db.utils import run_software_center_agent
 
 # To test, run with e.g.
-# dbus-send --session --type=method_call \
-#   --dest=com.ubuntu.SoftwareCenterDataProvider --print-reply \
-#   /com/ubuntu/SoftwareCenterDataProvider
-#   com.ubuntu.SoftwareCenterDataProvider.GetAppDetails string:"" string:"apt"
-#
+"""
+dbus-send --session --type=method_call \
+   --dest=com.ubuntu.SoftwareCenterDataProvider --print-reply \
+   /com/ubuntu/SoftwareCenterDataProvider \
+   com.ubuntu.SoftwareCenterDataProvider.GetAppDetails string:"" string:"apt"
+"""
 
 
 LOG = logging.getLogger(__file__)
 
+DBUS_BUS_NAME = 'com.ubuntu.SoftwareCenterDataProvider'
+DBUS_DATA_PROVIDER_IFACE = 'com.ubuntu.SoftwareCenterDataProvider'
+DBUS_DATA_PROVIDER_PATH = '/com/ubuntu/SoftwareCenterDataProvider'
+
 
 class SoftwareCenterDataProvider(dbus.service.Object):
 
-    def __init__(self, bus_name,
-                 object_path='/com/ubuntu/SoftwareCenterDataProvider'):
+    def __init__(self, bus_name, object_path=DBUS_DATA_PROVIDER_PATH):
         dbus.service.Object.__init__(self, bus_name, object_path)
         self.bus_name = bus_name
         # the database
@@ -67,14 +71,14 @@
         """ stop the dbus controller and remove from the bus """
         self.remove_from_connection()
 
-    @dbus.service.method('com.ubuntu.SoftwareCenterDataProvider',
+    @dbus.service.method(DBUS_DATA_PROVIDER_IFACE,
                          in_signature='ss', out_signature='a{sv}')
     def GetAppDetails(self, appname, pkgname):
         LOG.debug("GetAppDetails() called with ('%s', '%s')" % (
                 appname, pkgname))
         app = Application(appname, pkgname)
         appdetails = app.get_details(self.db)
-        return appdetails.as_property_dict()
+        return appdetails.as_dbus_property_dict()
 
     @dbus.service.method('com.ubuntu.SoftwareCenterDataProvider',
                          in_signature='', out_signature='as')
@@ -82,7 +86,7 @@
         LOG.debug("GetAvailableCategories() called")
         return [cat.name for cat in self.categories]
 
-    @dbus.service.method('com.ubuntu.SoftwareCenterDataProvider',
+    @dbus.service.method(DBUS_DATA_PROVIDER_IFACE,
                          in_signature='s', out_signature='as')
     def GetAvailableSubcategories(self, category_name):
         LOG.debug("GetAvailableSubcategories() called")
@@ -105,10 +109,10 @@
         return result
 
 
-def dbus_main():
-    bus = dbus.SessionBus()
-    bus_name = dbus.service.BusName(
-        'com.ubuntu.SoftwareCenterDataProvider', bus)
+def dbus_main(bus=None):
+    if bus is None:
+        bus = dbus.SessionBus()
+    bus_name = dbus.service.BusName(DBUS_BUS_NAME, bus)
     data_provider = SoftwareCenterDataProvider(bus_name)
     data_provider  # pyflakes
 
 
=== modified file 'tests/test_dataprovider.py'
--- tests/test_dataprovider.py	2012-09-12 12:41:05 +0000
+++ tests/test_dataprovider.py	2012-09-18 08:18:20 +0000
@@ -1,7 +1,14 @@
 
 import dbus
+import time
 import unittest
 
+from dbus.mainloop.glib import DBusGMainLoop
+DBusGMainLoop(set_as_default=True)
+
+from multiprocessing import Process
+from mock import Mock
+
 from tests.utils import (
     kill_process,
     setup_test_env,
@@ -9,11 +16,76 @@
 )
 setup_test_env()
 
-from softwarecenter.db.dataprovider import SoftwareCenterDataProvider
-
+from softwarecenter.db.application import AppDetails
+from softwarecenter.db.dataprovider import (
+    SoftwareCenterDataProvider,
+    dbus_main,
+    DBUS_BUS_NAME,
+    DBUS_DATA_PROVIDER_IFACE,
+    DBUS_DATA_PROVIDER_PATH,
+    )
+
+
+class DbusForRealTestCase(unittest.TestCase):
+    """Test the dataprovider over a real dbus bus"""
+
+    @classmethod
+    def setUpClass(cls):
+        cls.p = Process(target=dbus_main)
+        cls.p.start()
+        time.sleep(1)
+
+    @classmethod
+    def tearDown(cls):
+        cls.p.terminate()
+
+    def setUp(self):
+        bus = dbus.SessionBus()
+        obj = bus.get_object(bus_name=DBUS_BUS_NAME,
+                         object_path=DBUS_DATA_PROVIDER_PATH,
+                         follow_name_owner_changes=True)
+        self.proxy = dbus.Interface(object=obj,
+                               dbus_interface=DBUS_DATA_PROVIDER_IFACE)
+
+    def test_dbus_for_real(self):
+        result = self.proxy.GetAppDetails("", "gedit")
+        self.assertEqual(result["pkgname"], "gedit")
+
+
+class PropertyDictExceptionsTestCase(unittest.TestCase):
+    """Test that the exceptions in the AppDetails for the dbus properties
+       are handled correctly
+    """
+
+    def setUp(self):
+        self.mock_app_details = Mock(AppDetails)
+
+    def test_simple(self):
+        self.mock_app_details.name = "fake-app"
+        properties = AppDetails.as_dbus_property_dict(self.mock_app_details)
+        self.assertEqual(properties["name"], "fake-app")
+
+    def test_empty_dict_set(self):
+        self.mock_app_details.empty_set = set()
+        self.mock_app_details.empty_dict = {}
+        properties = AppDetails.as_dbus_property_dict(self.mock_app_details)
+        self.assertEqual(properties["empty_set"], "")
+        self.assertEqual(properties["empty_dict"], "")
+
+    def test_normal_dict(self):
+        self.mock_app_details.non_empty_dict = { "moo" : "bar" }
+        properties = AppDetails.as_dbus_property_dict(self.mock_app_details)
+        self.assertEqual(properties["non_empty_dict"], { "moo" : "bar" })
+
+    def test_normal_set(self):
+        self.mock_app_details.non_empty_set = set(["foo", "bar", "baz"])
+        properties = AppDetails.as_dbus_property_dict(self.mock_app_details)
+        self.assertEqual(
+            sorted(properties["non_empty_set"]), ["bar", "baz", "foo"])
 
 
 class DataProviderTestCase(unittest.TestCase):
+    """ Test the methods of the dataprovider """
 
     @classmethod
     def setUpClass(cls):
@@ -41,6 +113,11 @@
         self.assertEqual(result["pkgname"], "gedit")
         self.assertEqual(result["price"], "Free")
         self.assertEqual(result["raw_price"], "")
+        self.assertEqual(result["is_desktop_dependency"], True)
+
+    def test_get_details_non_desktop(self):
+        result = self.provider.GetAppDetails("", "apache2")
+        self.assertEqual(result["is_desktop_dependency"], False)
 
     # get available categories/subcategories
     def test_get_categories(self):
@@ -69,5 +146,8 @@
         result = self.provider.GetItemsForCategory(u"What\u2019s New")
         self.assertEqual(len(result), 20)
 
+
 if __name__ == "__main__":
+    #import logging
+    #logging.basicConfig(level=logging.DEBUG)
     unittest.main()