Merge lp:~mvo/software-center/recommender_uuid_consolidation into lp:software-center

[Michael Vogt]

This branch does some cleanup in the way we store the recommender_uuid.

All usage from app.py is removed as its currently not used or needed there. We only use it to write it in the config
but that can be done in the recagent.py as well.

The RecommenderAgent.recommender_uuid is a property to ensure its always in sync with the (global singelton)
SoftwareCenterConfig. This ensures that on close SoftwareCenterConfig.write() will include the recommender_uuid
value.

Plus a test to ensure that the property is calling the SoftwareCenterConfig in the expected way.

[Gary Lasker (gary-lasker)]

Hi mvo, thanks for this! I think it's a good idea to consolidate the UUID stuff more, however, there are two reasons why I left the connection to app.py (with the signal). The first is because I planned to add an opt-out menu item (which will be a dead-simple change). The second is that I thought it best to keep all of the code that writes to the main config file in one place, that is, in app.py. Else somebody else will have to search if they want to explain where this mysterious other field is coming from in the main config file.

In any case, I am seeing two problems with it the current branch. One is a failure in the unit test, the other occurs when running the app itself (specifically, soemthing that would occur during a first run in a new install, further details below).

First, the unit test:

======================================================================
ERROR: test_mocked_recagent_post_submit_profile (__main__.TestRecommenderAgent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_recagent.py", line 48, in test_mocked_recagent_post_submit_profile
    recommender_agent.post_submit_profile(db)
  File "/home/tremolux/Projects/precise/software-center_review_mvo_recommender_uuid_consolidation/software-center/softwarecenter/backend/recagent.py", line 98, in post_submit_profile
    self.recommender_uuid = get_uuid()
  File "/home/tremolux/Projects/precise/software-center_review_mvo_recommender_uuid_consolidation/software-center/softwarecenter/backend/recagent.py", line 236, in _set_recommender_uuid
    config.set("general", "recommender_uuid", self.recommender_uuid)
  File "/usr/lib/python2.7/ConfigParser.py", line 753, in set
    ConfigParser.set(self, section, option, value)
  File "/usr/lib/python2.7/ConfigParser.py", line 396, in set
    raise NoSectionError(section)
NoSectionError: No section: 'general'

======================================================================
FAIL: test_recagent_recommender_uuid_property (__main__.TestRecommenderAgent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_recagent.py", line 161, in test_recagent_recommender_uuid_property
    self.assertEqual(recommender_agent.recommender_uuid, "foxy-uuid")
AssertionError: '' != 'foxy-uuid'

----------------------------------------------------------------------
Ran 7 tests in 6.296s

FAILED (failures=1, errors=1)

---

And the failure with the app itself occurs in the case where we have not yet made the cfg file (you can see this with a 'rm .config/software-center/softwarecenter.cfg'). This would happen on first run of software-center for a new install, for instance, and if the user opt-in to recommendations then. What happens in the UI is that the "Uploading user profile" spinner hangs.

Traceback (most recent call last):
  File "/home/tremolux/Projects/precise/software-center_review_mvo_recommender_uuid_consolidation/software-center/softwarecenter/ui/gtk3/widgets/recommendations.py", line 127, in _on_opt_in_button_clicked
    self._upload_user_profile_and_get_recommendations()
  File "/home/tremolux/Projects/precis...

review: Needs Fixing

[Michael Vogt (mvo)]

I set this to rejected as it no longer applies with the recent changes.

Unmerged revisions

2799. By Michael Vogt on 2012-03-01

REFACTOR: remove recommender_uuid from app.py as it does not need to know about this (the widget is self contained currently and the state can set set using the global get_config() config that will saved on s-c exit); also make recommender_uuid a property so that reads/writes are automatically stored in the global s-c config

=== modified file 'softwarecenter/backend/recagent.py'
--- softwarecenter/backend/recagent.py	2012-02-28 20:27:50 +0000
+++ softwarecenter/backend/recagent.py	2012-03-01 09:12:23 +0000
@@ -75,8 +75,8 @@
     def __init__(self, xid=None):
         GObject.GObject.__init__(self)
         self.xid = xid
-        self.recommender_uuid = self._get_recommender_uuid()
 
+    # methods
     def query_server_status(self):
         # build the command
         spawner = SpawnHelper()
@@ -202,18 +202,6 @@
     def _on_recommend_top_data(self, spawner, piston_top_apps):
         self.emit("recommend-top", piston_top_apps)
         
-    def _get_recommender_uuid(self):
-        """ returns the recommender UUID value, which can be empty if it
-            has not yet been set (indicating that the user has not yet
-            opted-in to the recommender service)
-        """
-        config = get_config()
-        if config.has_option("general", "recommender_uuid"):
-            recommender_uuid = config.get("general", "recommender_uuid")
-            if recommender_uuid:
-                return recommender_uuid
-        return ""
-        
     def _generate_submit_profile_data(self, recommender_uuid, package_list):
         submit_profile_data = [
             {
@@ -223,6 +211,36 @@
         ]
         return submit_profile_data
 
+    # properties
+    def _get_recommender_uuid(self):
+        """ returns the recommender UUID value, which can be empty if it
+            has not yet been set (indicating that the user has not yet
+            opted-in to the recommender service)
+        """
+        if not hasattr(self, "_recommender_uuid"):
+            config = get_config()
+            if config.has_option("general", "recommender_uuid"):
+                self._recommender_uuid = config.get(
+                    "general", "recommender_uuid")
+            else:
+                self._recommender_uuid = ""
+        return self._recommender_uuid
+        
+        return self._recommender_uuid
+    def _set_recommender_uuid(self, value):
+        """ this sets the recommender UUID value, both internally for
+            this class and in the app wide software-center config
+        """
+        self._recommender_uuid = value
+        config = get_config()
+        config.set("general", "recommender_uuid", self.recommender_uuid)
+    recommender_uuid = property(
+        _get_recommender_uuid, _set_recommender_uuid, None, 
+        """ The (persistent) recommender_uuid property that is stored
+            in the config under general/recommender_uuid
+        """)
+
+
    
 if __name__ == "__main__":
     from gi.repository import Gtk

=== modified file 'softwarecenter/ui/gtk3/app.py'
--- softwarecenter/ui/gtk3/app.py	2012-02-29 17:23:24 +0000
+++ softwarecenter/ui/gtk3/app.py	2012-03-01 09:12:23 +0000
@@ -258,7 +258,6 @@
         self.scagent = None
         self.sso = None
         self.available_for_me_query = None
-        self.recommender_uuid = ""
 
         Gtk.Window.set_default_icon_name("softwarecenter")
 
@@ -468,25 +467,6 @@
 
     def on_available_pane_created(self, widget):
         self.available_pane.searchentry.grab_focus()
-        # connect a signal to monitor the recommendations opt-in state and
-        # persist the recommendations uuid on an opt-in
-        self.available_pane.cat_view.recommended_for_you_panel.connect(
-                        "recommendations-opt-in",
-                        self._on_recommendations_opt_in)
-        self.available_pane.cat_view.recommended_for_you_panel.connect(
-                        "recommendations-opt-out",
-                        self._on_recommendations_opt_out)
-    
-    #~ def on_installed_pane_created(self, widget):
-        #~ pass
-        
-    def _on_recommendations_opt_in(self, agent, recommender_uuid):
-        self.recommender_uuid = recommender_uuid
-    
-    def _on_recommendations_opt_out(self):
-        # if the user opts back out of the recommender service, we
-        # reset the UUID to indicate it
-        self.recommender_uuid = ""
     
     def _on_update_software_center_agent_finished(self, pid, condition):
         LOG.info("software-center-agent finished with status %i" % os.WEXITSTATUS(condition))
@@ -1231,9 +1211,6 @@
         else:
             # initial default state is to add to launcher, per spec
             self.available_pane.add_to_launcher_enabled = True
-        if self.config.has_option("general", "recommender_uuid"):
-            self.recommender_uuid = self.config.get("general",
-                                                    "recommender_uuid")
 
     def save_state(self):
         LOG.debug("save_state")
@@ -1255,9 +1232,6 @@
             self.config.set("general", "add_to_launcher", "True")
         else:
             self.config.set("general", "add_to_launcher", "False")
-        self.config.set("general",
-                        "recommender_uuid",
-                        self.recommender_uuid)
         self.config.write()
 
     def run(self, args):

=== modified file 'softwarecenter/ui/gtk3/widgets/recommendations.py'
--- softwarecenter/ui/gtk3/widgets/recommendations.py	2012-02-29 19:58:05 +0000
+++ softwarecenter/ui/gtk3/widgets/recommendations.py	2012-03-01 09:12:23 +0000
@@ -110,6 +110,13 @@
         label.set_alignment(0, 0.5)
         label.set_line_wrap(True)
         vbox.pack_start(label, False, False, 0)
+
+    # FIXME: there is no opt-out button yet!
+    def _on_opt_out_button_clicked(self, button):
+        # if the user opts back out of the recommender service, we
+        # reset the UUID to indicate it
+        self.recommender_agent.recommender_uuid = ""
+        self.emit("recommendations-opt-out")
         
     def _on_opt_in_button_clicked(self, button):
         # we upload the user profile here, and only after this is finished

=== modified file 'test/test_recagent.py'
--- test/test_recagent.py	2012-03-01 00:14:51 +0000
+++ test/test_recagent.py	2012-03-01 09:12:23 +0000
@@ -152,6 +152,19 @@
         self.loop.run()
         self.assertTrue(self.error)
 
+    @patch('softwarecenter.config.SoftwareCenterConfig')
+    def test_recagent_recommender_uuid_property(self, mock_config_klass):
+        mock_config = mock_config_klass.return_value
+        mock_config.has_option.return_value = True
+        mock_config.get.return_value = "foxy-uuid"
+        recommender_agent = RecommenderAgent()
+        self.assertEqual(recommender_agent.recommender_uuid, "foxy-uuid")
+        recommender_agent.recommender_uuid = "new-value"
+        self.assertEqual(recommender_agent.recommender_uuid, "new-value")
+        self.assertTrue(mock_config.set.called)
+        mock_config.set.assertCalledWith("general","recommender_uuid",
+                                         "new-value")
+
 if __name__ == "__main__":
     #import logging
     #logging.basicConfig(level=logging.DEBUG)