Merge lp:~mvo/software-center/unity-lens-plus-wildcard-fix into lp:software-center

[Michael Vogt]

This branch adds a "Dash Search Plugins" subcategory to Tweaks&Themes.
It also adds a workaround for a bug that the xapian query parser will
not work if there is a "-" in the query term

[Gary Lasker (gary-lasker)]

Hi Michael, and thanks for doing this work. So, I can see the "Dash Search Plugins" subcategory in the Tweaks&Themes section, and it shows two lenses - the AskUbuntu lens and a Wikipedia lens. However, if I search all categories for the string "unity-lens-" I can actually see quite a few more lenses that are hidden in the technical items. I'm not quite sure how we want to fix this, or even if it is a problem. It seems, however, that we'll want all of the lenses to appear in the subcategory. Please let me know what you think.

Also, could you please describe just a little more detail about the Xapian workaround you added? Is there a bug for this issue by chance? Maybe you could provide a test case to demonstrate how the error is manifested?

Finally, I am getting an error in the new unit test that you added to test_xapian.py. It seems I'm not getting any results back for the query. Possibly it's something simple on my end. Please let me know. Here's the error:

PYTHONPATH=. python tests/test_xapian.py
F......No handlers could be found for logger "softwarecenter.distro.ubuntu"
.
======================================================================
FAIL: test_wildcard_bug1025579_workaround (__main__.AptXapianIndexTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/test_xapian.py", line 126, in test_wildcard_bug1025579_workaround
    self.assertNotEqual(len(mset), 0)
AssertionError: 0 == 0

----------------------------------------------------------------------
Ran 8 tests in 0.097s

FAILED (failures=1)

Many thanks!

[Gary Lasker (gary-lasker)]

This code looks good and I'll set it as approved so as not to block it. We can tweak if needed/as needed in future branches, and this gets us the feature which is excellent. Michael, please just take a look at the unit test issue. I'm sure it's passing on your machine so I'm probably just missing something on my end.

Thanks again for this! It's great to have this for FF.

review: Approve

[Michael Vogt (mvo)]

Thanks for the review Gary. Its probably failing for you because you are not using the current quantal version of apt-xapian-index that I uploaded to fix the same problem in there too (that "-" is special). I will add a test skip decorator probably that tests for the version of a-x-i.

[lp:~mvo/software-center/unity-lens-plus-wildcard-fix]

3109. By Michael Vogt on 2012-08-21

tests/test_xapian.py: skip test for wildcard bug1026679 as the fix was added in quantals apt-xapian-index only

3110. By Michael Vogt on 2012-08-21

update db schema because we have the new APM key and add test for ensuring that the APM values are setup correclty

3111. By Michael Vogt on 2012-08-21

use xapian.inmemory_open() for DB tests that do not need to be persistent

3112. By Michael Vogt on 2012-08-21

add explicit testcase for the xapian query parser "-" workaround/bug

3113. By Michael Vogt on 2012-08-21

tests/test_database.py: reference the upstream xapian ticket

3114. By Michael Vogt on 2012-08-21

apt_xapian_index_plugin/software_center.py: include package name terms in a-x-i plugin too

3115. By Michael Vogt on 2012-08-21

make sure category flags are applied for subcategories as well

3116. By Michael Vogt on 2012-08-21

data/software-center.menu.in: add nonapps-visible flag to unity-lens

[Michael Vogt (mvo)]

On Sat, Aug 18, 2012 at 06:39:17AM -0000, Gary Lasker wrote:
> Hi Michael, and thanks for doing this work. So, I can see the "Dash Search Plugins" subcategory in the Tweaks&Themes section, and it shows two lenses - the AskUbuntu lens and a Wikipedia lens. However, if I search all categories for the string "unity-lens-" I can actually see quite a few more lenses that are hidden in the technical items. I'm not quite sure how we want to fix this, or even if it is a problem. It seems, however, that we'll want all of the lenses to appear in the subcategory. Please let me know what you think.

Thanks for the review. There was a flag missing to always make the
nonapps visible. There was also a bug in the subcategory part of the
available pane that prevented the flag from getting used. This should
all be fixed now.

> Also, could you please describe just a little more detail about the Xapian workaround you added? Is there a bug for this issue by chance? Maybe you could provide a test case to demonstrate how the error is manifested?

I added a TestCase for tihs now, it includes a link to the quoting
problem of the xapian query parser (http://trac.xapian.org/ticket/128)

> Finally, I am getting an error in the new unit test that you added to test_xapian.py. It seems I'm not getting any results back for the query. Possibly it's something simple on my end. Please let me know. Here's the error:
[..]

This seems to be caused by running on precise, I exclude this test
now if precise is used, there is no "XPM" term in the xapian db on
precise.

Cheers and enjoy your vacation.

Cheers,
 Michael

[lp:~mvo/software-center/unity-lens-plus-wildcard-fix]

3117. By Michael Vogt on 2012-08-21

pep8 fixes

=== modified file 'apt_xapian_index_plugin/software_center.py'
--- apt_xapian_index_plugin/software_center.py	2012-07-02 09:51:31 +0000
+++ apt_xapian_index_plugin/software_center.py	2012-08-21 13:19:20 +0000
@@ -10,7 +10,10 @@
     CustomKeys,
     XapianValues,
     )
-from softwarecenter.db.update import WEIGHT_DESKTOP_NAME
+from softwarecenter.db.update import (
+    WEIGHT_DESKTOP_NAME,
+    get_pkgname_terms,
+    )
 from softwarecenter.distro import get_distro
 
 
@@ -104,6 +107,8 @@
             # add s-c values/terms for the name
             document.add_term("AA"+name)
             document.add_value(XapianValues.APPNAME, name)
+            for t in get_pkgname_terms(pkg.name):
+                document.add_term(t)
             self.indexer.index_text_without_positions(
                 name, WEIGHT_DESKTOP_NAME)
             # we pretend to be an application

=== modified file 'data/software-center.menu.in'
--- data/software-center.menu.in	2012-05-15 02:39:12 +0000
+++ data/software-center.menu.in	2012-08-21 13:19:20 +0000
@@ -332,16 +332,13 @@
     <Include>
       <And>
         <Or>
-          <SCPkgnameWildcard>ttf*</SCPkgnameWildcard>
-          <SCPkgnameWildcard>otf*</SCPkgnameWildcard>
+          <SCPkgnameWildcard>ttf-*</SCPkgnameWildcard>
+          <SCPkgnameWildcard>otf-*</SCPkgnameWildcard>
           <SCSection>fonts</SCSection>
           <SCSection>restricted/fonts</SCSection>
           <SCSection>universe/fonts</SCSection>
           <SCSection>multiverse/fonts</SCSection>
         </Or>
-        <Not>
-          <SCPkgname>ttfm</SCPkgname>
-        </Not>
       </And>
     </Include>
   </Menu>
@@ -528,7 +525,24 @@
   <Menu>
     <_Name>Themes &amp; Tweaks</_Name>
     <SCIcon>preferences-other</SCIcon>
-    <Include><Category>Settings</Category></Include>
+    <Include>
+      <Category>Settings</Category>
+    </Include>
+
+    <!-- Sub-categories for the Themes -->
+    <Menu>
+      <_Name>Dash Search Plugins</_Name>
+      <Flags>
+        <Flag>nonapps-visible</Flag>
+      </Flags>
+      <SCIcon>preferences-other</SCIcon>
+      <Include>
+        <And>
+          <SCPkgnameWildcard>unity-lens-*</SCPkgnameWildcard>
+        </And>
+      </Include>
+    </Menu>
+    
   </Menu>
 
  <!-- System -->

=== modified file 'setup.py'
--- setup.py	2012-07-23 11:48:24 +0000
+++ setup.py	2012-08-21 13:19:20 +0000
@@ -69,10 +69,10 @@
 DISTRO = platform.dist()[0]
 RELEASE = platform.dist()[1]
 open("softwarecenter/version.py", "w").write("""
-VERSION='%s'
-CODENAME='%s'
-DISTRO='%s'
-RELEASE='%s'
+VERSION = '%s'
+CODENAME = '%s'
+DISTRO = '%s'
+RELEASE = '%s'
 """ % (VERSION, CODENAME, DISTRO, RELEASE))
 
 

=== modified file 'softwarecenter/db/categories.py'
--- softwarecenter/db/categories.py	2012-08-17 07:46:59 +0000
+++ softwarecenter/db/categories.py	2012-08-21 13:19:20 +0000
@@ -399,8 +399,10 @@
                 query = xapian.Query(xapian_op, query, q)
             elif operator_elem.tag == "SCPkgnameWildcard":
                 LOG.debug("adding tag: %s" % operator_elem.text)
-                # query both axi and s-c
-                s = "pkg_wildcard:%s" % qtext
+                # query both axi and s-c and ensure that the pkgname is
+                # mangled to workaround xapians query parser lack of
+                # quoting :/
+                s = "pkg_wildcard:%s" % qtext.replace("-", "_")
                 q = self.db.xapian_parser.parse_query(s,
                     xapian.QueryParser.FLAG_WILDCARD)
                 query = xapian.Query(xapian_op, query, q)

=== modified file 'softwarecenter/db/database.py'
--- softwarecenter/db/database.py	2012-08-14 14:45:47 +0000
+++ softwarecenter/db/database.py	2012-08-21 13:19:20 +0000
@@ -204,7 +204,9 @@
         xapian_parser.add_boolean_prefix("section", "XS")
         xapian_parser.add_boolean_prefix("origin", "XOC")
         xapian_parser.add_prefix("pkg_wildcard", "XP")
+        xapian_parser.add_prefix("pkg_wildcard", "XPM")
         xapian_parser.add_prefix("pkg_wildcard", "AP")
+        xapian_parser.add_prefix("pkg_wildcard", "APM")
         xapian_parser.set_default_op(xapian.Query.OP_AND)
         return xapian_parser
 

=== modified file 'softwarecenter/db/update.py'
--- softwarecenter/db/update.py	2012-08-01 04:37:06 +0000
+++ softwarecenter/db/update.py	2012-08-21 13:19:20 +0000
@@ -118,10 +118,10 @@
 
 
 def get_pkgname_terms(pkgname):
-    result = ["AP" + pkgname]
-    if '-' in pkgname:
-        # we need this to work around xapian oddness
-        result.append(pkgname.replace('-', '_'))
+    result = ["AP" + pkgname,
+              # workaround xapian oddness by providing a "mangled" version
+              # with a different prefix
+              "APM" + pkgname.replace('-', '_')]
     return result
 
 
@@ -239,7 +239,7 @@
                 doc_key = self.FIELD_TO_XAPIAN[key]
                 doc.add_value(doc_key, value)
                 # add terms to the xapian database
-                get_terms = self.FIELD_TO_TERMS.get(key, lambda i: i)
+                get_terms = self.FIELD_TO_TERMS.get(key, lambda i: [])
                 for t in get_terms(value):
                     doc.add_term(t)
 

=== modified file 'softwarecenter/enums.py'
--- softwarecenter/enums.py	2012-08-01 04:26:27 +0000
+++ softwarecenter/enums.py	2012-08-21 13:19:20 +0000
@@ -61,7 +61,7 @@
 
 # version of the database, every time something gets added (like
 # terms for mime-type) increase this (but keep as a string!)
-DB_SCHEMA_VERSION = "6"
+DB_SCHEMA_VERSION = "7"
 
 # the default limit for a search
 DEFAULT_SEARCH_LIMIT = 10000
@@ -253,9 +253,9 @@
 
 # visibility of non applications in the search results
 class NonAppVisibility:
-    (ALWAYS_VISIBLE,
-     MAYBE_VISIBLE,
-     NEVER_VISIBLE) = range(3)
+    ALWAYS_VISIBLE = "non-apps-always-visible"
+    MAYBE_VISIBLE = "non-apps-maybe-visible"
+    NEVER_VISIBLE = "non-apps-never-visible"
 
 
 # application actions

=== modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
--- softwarecenter/ui/gtk3/panes/availablepane.py	2012-07-19 09:37:04 +0000
+++ softwarecenter/ui/gtk3/panes/availablepane.py	2012-08-21 13:19:20 +0000
@@ -696,7 +696,9 @@
 
     def display_app_view_page(self, view_state):
         category = view_state.category
+        subcategory = view_state.subcategory
         self.set_category(category)
+        self.set_subcategory(subcategory)
 
         result = self.display_list_page(view_state)
 
@@ -790,10 +792,17 @@
         current_page = self.notebook.get_current_page()
         return current_page == self.Pages.PURCHASE
 
+    def set_subcategory(self, subcategory):
+        LOG.debug('set_subcategory: %s' % subcategory)
+        self.state.subcategory = subcategory
+        self._apply_filters_for_category_or_subcategory(subcategory)
+
     def set_category(self, category):
         LOG.debug('set_category: %s' % category)
         self.state.category = category
+        self._apply_filters_for_category_or_subcategory(category)
 
+    def _apply_filters_for_category_or_subcategory(self, category):
         # apply flags
         if category:
             if 'nonapps-visible' in category.flags:

=== modified file 'softwarecenter/utils.py'
--- softwarecenter/utils.py	2012-08-16 14:44:29 +0000
+++ softwarecenter/utils.py	2012-08-21 13:19:20 +0000
@@ -663,9 +663,8 @@
         # set new global datadir
         softwarecenter.paths.datadir = datadir
         # also alter the app-install path
-        path = "%s/desktop/software-center.menu" % \
-            softwarecenter.paths.APP_INSTALL_PATH
-        if not os.path.exists(path):
+        path = "./build/share/app-install/desktop/software-center.menu"
+        if os.path.exists(path):
             softwarecenter.paths.APP_INSTALL_PATH = './build/share/app-install'
             logging.warn("using local APP_INSTALL_PATH: %s" %
                              softwarecenter.paths.APP_INSTALL_PATH)

=== modified file 'softwarecenter/version.py'
--- softwarecenter/version.py	2012-08-01 04:37:06 +0000
+++ softwarecenter/version.py	2012-08-21 13:19:20 +0000
@@ -1,5 +1,5 @@
 
-VERSION = '5.3.3.1'
-CODENAME = 'quantal'
+VERSION = '5.3.7'
+CODENAME = 'UNRELEASED'
 DISTRO = 'Ubuntu'
 RELEASE = '12.10'

=== modified file 'tests/test_database.py'
--- tests/test_database.py	2012-08-15 12:11:14 +0000
+++ tests/test_database.py	2012-08-21 13:19:20 +0000
@@ -13,6 +13,7 @@
 from tests.utils import (
     DATA_DIR,
     get_test_db,
+    get_test_db_from_app_install_data,
     get_test_pkg_info,
     do_events,
     make_software_center_agent_subscription_dict,
@@ -77,24 +78,26 @@
         self.assertEqual(res, [ v2, v1, v0 ])
 
 
+    def _get_db_from_test_app_install_data(self):
+        db = xapian.inmemory_open()
+        res = update_from_app_install_data(db, self.cache,
+            datadir=os.path.join(DATA_DIR, "desktop"))
+        self.assertTrue(res)
+        self.assertEqual(db.get_doccount(), 5)
+        return db
+
     def test_update_from_desktop_file(self):
         # ensure we index with german locales to test i18n
         os.environ["LANGUAGE"] = "de"
-        db = xapian.WritableDatabase(TEST_DB,
-                                     xapian.DB_CREATE_OR_OVERWRITE)
-        res = update_from_app_install_data(db, self.cache,
-            datadir=os.path.join(DATA_DIR, "desktop"))
-        self.assertTrue(res)
-        self.assertEqual(db.get_doccount(), 5)
+        datadir = os.path.join(DATA_DIR, "desktop")
+        db = get_test_db_from_app_install_data(datadir)
         # test if Name[de] was picked up
         i=0
         for it in db.postlist("AAUbuntu Software Zentrum"):
             i+=1
-        self.assertEqual(i, 1)
 
     def test_update_from_appstream_xml(self):
-        db = xapian.WritableDatabase(TEST_DB,
-                                     xapian.DB_CREATE_OR_OVERWRITE)
+        db = xapian.inmemory_open()
         res = update_from_appstream_xml(db, self.cache,
             os.path.join(DATA_DIR, "app-info"))
         self.assertTrue(res)
@@ -113,8 +116,7 @@
     def test_update_from_var_lib_apt_lists(self):
         # ensure we index with german locales to test i18n
         os.environ["LANGUAGE"] = "de"
-        db = xapian.WritableDatabase(TEST_DB,
-                                     xapian.DB_CREATE_OR_OVERWRITE)
+        db = xapian.inmemory_open()
         res = update_from_var_lib_apt_lists(db, self.cache,
             listsdir=os.path.join(DATA_DIR, "app-info"))
         self.assertTrue(res)
@@ -137,8 +139,7 @@
         self.assertTrue(found_gettext_translation)
 
     def test_update_from_json_string(self):
-        db = xapian.WritableDatabase(TEST_DB,
-                                     xapian.DB_CREATE_OR_OVERWRITE)
+        db = xapian.inmemory_open()
         cache = apt.Cache()
         p = os.path.join(DATA_DIR, "app-info-json", "apps.json")
         res = update_from_json_string(db, cache, open(p).read(), origin=p)
@@ -146,8 +147,7 @@
         self.assertEqual(db.get_doccount(), 1)
 
     def test_build_from_software_center_agent(self):
-        db = xapian.WritableDatabase(TEST_DB,
-                                     xapian.DB_CREATE_OR_OVERWRITE)
+        db = xapian.inmemory_open()
         cache = apt.Cache()
         # monkey patch distro to ensure we get data
         distro = softwarecenter.distro.get_distro()
@@ -343,8 +343,7 @@
         # staging does not have a valid cert
         os.environ["PISTON_MINI_CLIENT_DISABLE_SSL_VALIDATION"] = "1"
         cache = get_test_pkg_info()
-        db = xapian.WritableDatabase(TEST_DB,
-                                     xapian.DB_CREATE_OR_OVERWRITE)
+        db = xapian.inmemory_open()
         res = update_from_software_center_agent(db, cache, ignore_cache=True)
         self.assertTrue(res)
 
@@ -702,6 +701,47 @@
         self.assertEqual(app.archive_suite, "")
 
 
+class XapianQueryParserWorkarounds(unittest.TestCase):
+    """This TestCase demonstrates the issues around the query
+       parser wildcard support if the "-" char is part of the 
+       pkgname and tests the workaround for this
+
+       (http://trac.xapian.org/ticket/128)
+    """
+
+    def setUp(self):
+        datadir = os.path.join(DATA_DIR, "desktop")
+        self.db = get_test_db_from_app_install_data(datadir)
+
+    def test_name_mangling_for_query_parser(self):
+        # test that pkgnames with "-" get added in a mangled form
+        i=0
+        for it in self.db.postlist("APMsoftware_center"):
+            i+=1
+        self.assertEqual(i, 1)
+
+    def test_query_parser_wildcard(self):
+        enquire = xapian.Enquire(self.db)
+        parser = xapian.QueryParser()
+        parser.set_database(self.db)
+        parser.add_prefix("pkg_wildcard", "AP")
+        # this demonstrates the xapian bug with the query parser 
+        # and "-" special chars, note that once this test fails (i.e.
+        # the returned mset is "1" we can remove this workaround
+        query = parser.parse_query(
+            "pkg_wildcard:software-*", xapian.QueryParser.FLAG_WILDCARD)
+        enquire.set_query(query)
+        mset = enquire.get_mset(0, 100)
+        self.assertEqual(len(mset), 0)
+        # and the workaround
+        parser.add_prefix("pkg_wildcard", "APM")
+        query = parser.parse_query(
+            "pkg_wildcard:software_*", xapian.QueryParser.FLAG_WILDCARD)
+        enquire.set_query(query)
+        mset = enquire.get_mset(0, 100)
+        self.assertEqual(len(mset), 1)
+
+
 class TrackDBTestCase(unittest.TestCase):
 
     def test_track_db_open(self):

=== modified file 'tests/test_xapian.py'
--- tests/test_xapian.py	2012-07-02 09:51:31 +0000
+++ tests/test_xapian.py	2012-08-21 13:19:20 +0000
@@ -1,5 +1,6 @@
+import os
+import platform
 import unittest
-import os
 import xapian
 
 from mock import Mock, patch
@@ -103,6 +104,31 @@
         #print len(matches)
         self.assertTrue(len(matches) > 0)
 
+
+class AptXapianIndexTestCase(unittest.TestCase):
+
+    # this will fail on precise so we skip the test there
+    @unittest.skipIf(platform.dist()[2] == "precise" and
+                     not os.path.exists("/var/lib/apt-xapian-index/index"),
+                     "Need populated apt-xapian-index for this test")
+    def test_wildcard_bug1025579_workaround(self):
+        db = xapian.Database("/var/lib/apt-xapian-index/index")
+        enquire = xapian.Enquire(db)
+        parser = xapian.QueryParser()
+        parser.set_database(db)
+        # this is the gist, the mangled version of the XPM term
+        parser.add_prefix("pkg_wildcard", "XPM")
+        parser.add_prefix("pkg_wildcard", "XP")
+        parser.add_prefix("pkg_wildcard", "AP")
+        s = 'pkg_wildcard:unity_lens_*'
+        query = parser.parse_query(s, xapian.QueryParser.FLAG_WILDCARD)
+        enquire.set_query(query)
+        mset = enquire.get_mset(0, 100)
+        for m in mset:
+            self.assertTrue(m.document.get_data().startswith("unity-lens-"))
+        self.assertNotEqual(len(mset), 0)
+
+
 class XapianPluginsTestCase(unittest.TestCase):
 
     def make_mock_document(self):
@@ -146,5 +172,6 @@
                 got_values.add(args[0])
             self.assertTrue(expected_values.issubset(got_values))
 
+
 if __name__ == "__main__":
     unittest.main()

=== modified file 'tests/utils.py'
--- tests/utils.py	2012-05-31 12:50:01 +0000
+++ tests/utils.py	2012-08-21 13:19:20 +0000
@@ -44,6 +44,8 @@
 )
 from softwarecenter.ui.gtk3.utils import get_sc_icon_theme
 from softwarecenter.utils import get_uuid
+from softwarecenter.db.update import update_from_app_install_data
+
 
 m_dbus = m_polkit = m_aptd = None
 
@@ -107,6 +109,16 @@
     return db
 
 
+def get_test_db_from_app_install_data(datadir):
+    db = xapian.inmemory_open()
+    cache = get_pkg_info()
+    cache.open()
+    res = update_from_app_install_data(db, cache, datadir)
+    if res is False:
+        raise AssertionError("Failed to build db from '%s'" % datadir)
+    return db
+
+
 def get_test_install_backend():
     backend = get_install_backend()
     return backend