[extensions-web] extensions: Fix base shell-version list visibility

[Jasper St. Pierre <jstpierre src gnome org>]

commit daa1be5d13431408d2eb55d268b256e8f40d7647
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Fri Mar 2 20:28:20 2012 -0500

    extensions: Fix base shell-version list visibility
    
    If someone queries for a non-existant "3.2.5" version, we need to query the
    base version "3.2". Since we had a nice object-oriented API to grab the base
    version, this wouldn't work, since we had no object for "3.2.5" to query the
    base version off of. Fall back to explicit querying, and remove the OOP API
    so we don't make that mistake again.

 sweettooth/extensions/models.py |   76 +++++++++++++-------------------------
 sweettooth/extensions/tests.py  |   38 +++++++++++---------
 sweettooth/extensions/views.py  |   25 ++++++++++---
 3 files changed, 66 insertions(+), 73 deletions(-)
---
diff --git a/sweettooth/extensions/models.py b/sweettooth/extensions/models.py
index b2c084b..179cc24 100644
--- a/sweettooth/extensions/models.py
+++ b/sweettooth/extensions/models.py
@@ -168,46 +168,47 @@ class ExtensionPopularityItem(models.Model):
 class InvalidShellVersion(Exception):
     pass
 
-class ShellVersionManager(models.Manager):
-    def parse_version_string(self, version_string, ignore_micro):
-        version = version_string.split('.')
+
+def parse_version_string(version_string, ignore_micro):
+    version = version_string.split('.')
+
+    try:
         major, minor = version[:2]
+        major, minor = int(major), int(minor)
+    except ValueError, e:
+        raise InvalidShellVersion()
+
+    if ignore_micro:
+        valid_lengths = (3, 4)
+    else:
+        valid_lengths = (3,)
 
+    if len(version) in valid_lengths:
+        # 3.0.1, 3.1.4
         try:
-            major, minor = int(major), int(minor)
+            point = int(version[2])
         except ValueError, e:
             raise InvalidShellVersion()
 
-        if ignore_micro:
-            valid_lengths = (3, 4)
-        else:
-            valid_lengths = (3,)
+    elif len(version) == 2 and minor % 2 == 0:
+        # 3.0, 3.2
+        point = -1
+    else:
+        # Two-digit odd versions are illegal: 3.1, 3.3
+        raise InvalidShellVersion()
 
-        if len(version) in valid_lengths:
-            # 3.0.1, 3.1.4
-            try:
-                point = int(version[2])
-            except ValueError, e:
-                raise InvalidShellVersion()
-
-        elif len(version) == 2 and minor % 2 == 0:
-            # 3.0, 3.2
-            point = -1
-        else:
-            # Two-digit odd versions are illegal: 3.1, 3.3
-            raise InvalidShellVersion()
-
-        return major, minor, point
+    return major, minor, point
 
+class ShellVersionManager(models.Manager):
     def lookup_for_version_string(self, version_string, ignore_micro=False):
-        major, minor, point = self.parse_version_string(version_string, ignore_micro)
+        major, minor, point = parse_version_string(version_string, ignore_micro)
         try:
             return self.get(major=major, minor=minor, point=point)
         except self.model.DoesNotExist:
             return None
 
     def get_for_version_string(self, version_string):
-        major, minor, point = self.parse_version_string(version_string, ignore_micro=False)
+        major, minor, point = parse_version_string(version_string, ignore_micro=False)
         obj, created = self.get_or_create(major=major, minor=minor, point=point)
         return obj
 
@@ -230,31 +231,6 @@ class ShellVersion(models.Model):
 
         return "%d.%d.%d" % (self.major, self.minor, self.point)
 
-    @property
-    def is_unstable(self):
-        return self.minor % 2 != 0
-
-    @property
-    def is_stable(self):
-        return self.minor % 2 == 0
-
-    @property
-    def base_version(self):
-        if self.point == -1:
-            return None
-
-        if self.is_unstable:
-            return None
-
-        try:
-            # This is intended to be use to easily query for all extensions
-            # that are compatible with a specific shell version. As such,
-            # this doesn't use get_or_create -- no sense creating a shell
-            # version that no extensions are going to match.
-            return self._default_manager.get(major=self.major, minor=self.minor, point=-1)
-        except self.DoesNotExist:
-            return None
-
 class InvalidExtensionData(Exception):
     pass
 
diff --git a/sweettooth/extensions/tests.py b/sweettooth/extensions/tests.py
index 6129052..a2067e3 100644
--- a/sweettooth/extensions/tests.py
+++ b/sweettooth/extensions/tests.py
@@ -254,32 +254,15 @@ class ShellVersionTest(TestCase):
         self.assertEquals(version.major, 3)
         self.assertEquals(version.minor, 0)
         self.assertEquals(version.point, 0)
-        self.assertTrue(version.is_stable)
 
         version = get_version("3.2")
         self.assertEquals(version.major, 3)
         self.assertEquals(version.minor, 2)
         self.assertEquals(version.point, -1)
-        self.assertTrue(version.is_stable)
 
         with self.assertRaises(models.InvalidShellVersion):
             version = get_version("3.1")
 
-        # Test that we don't track shell versions for unstable extensions
-        # (it doesn't really matter because we can't really create a base
-        # shell version for unstable extensions *anyway*, but oh well)
-        version = get_version("3.1.4")
-        self.assertEquals(version.base_version, None)
-
-        # Test that we don't create shell versions
-        version = get_version("3.4.1")
-        self.assertEquals(lookup_version("3.4"), None)
-        self.assertEquals(version.base_version, None)
-
-        # OK, go and create the base version and make sure we track it
-        base_version = get_version("3.4")
-        self.assertEquals(version.base_version, base_version)
-
 class UpdateVersionTest(TestCase):
     fixtures = [os.path.join(testdata_dir, 'test_upgrade_data.json')]
 
@@ -395,3 +378,24 @@ class QueryExtensionsTest(BasicUserTestCase, TestCase):
         # And now that we have a new version on two, we should have both...
         uuids = self.gather_uuids(dict(uuid=[one.uuid, two.uuid]))
         self.assertEqual(uuids, [one.uuid, two.uuid])
+
+    def test_shell_versions(self):
+        one = self.create_extension("one")
+        two = self.create_extension("two")
+
+        v = models.ExtensionVersion.objects.create(extension=one, status=models.STATUS_ACTIVE)
+        v.parse_metadata_json({"shell-version": ["3.2"]})
+
+        v = models.ExtensionVersion.objects.create(extension=two, status=models.STATUS_ACTIVE)
+        v.parse_metadata_json({"shell-version": ["3.3.90"]})
+
+        # Basic querying...
+        uuids = self.gather_uuids(dict(shell_version="3.2"))
+        self.assertEqual(uuids, [one.uuid])
+
+        uuids = self.gather_uuids(dict(shell_version="3.3.90"))
+        self.assertEqual(uuids, [two.uuid])
+
+        # Base version querying.
+        uuids = self.gather_uuids(dict(shell_version="3.2.2"))
+        self.assertEqual(uuids, [one.uuid])
diff --git a/sweettooth/extensions/views.py b/sweettooth/extensions/views.py
index 524a2b2..67b1817 100644
--- a/sweettooth/extensions/views.py
+++ b/sweettooth/extensions/views.py
@@ -74,16 +74,29 @@ def shell_update(request):
     return operations
 
 def get_versions_for_version_strings(version_strings):
+    def get_version(major, minor, point):
+        try:
+            return models.ShellVersion.objects.get(major=major, minor=minor, point=point)
+        except models.ShellVersion.DoesNotExist:
+            return None
+
     for version_string in version_strings:
-        version = models.ShellVersion.objects.lookup_for_version_string(version_string, ignore_micro=True)
-        if version is None:
+        try:
+            major, minor, point = models.parse_version_string(version_string, ignore_micro=True)
+        except models.InvalidShellVersion:
             continue
-        yield version
 
-        base_version = version.base_version
-        if base_version is None:
+        version = get_version(major, minor, point)
+        if version:
+            yield version
+
+        # If we already have a base version, don't bother querying it again...
+        if point == -1:
             continue
-        yield base_version
+
+        base_version = get_version(major, minor, -1)
+        if base_version:
+            yield base_version
 
 def ajax_query_params_query(request):
     query_params = {}