Move version parsing routimes to more appropriate spec.py; add tests

4 files changed, 208 insertions, 99 deletions

diff --git a/sugar_network/client/solver.py b/sugar_network/client/solver.py
index 1892aef..14816a6 100644
--- a/sugar_network/client/solver.py
+++ b/sugar_network/client/solver.py
@@ -18,7 +18,8 @@ import logging
 from os.path import isabs, join, dirname
 
 from sugar_network.client import packagekit, SUGAR_API_COMPATIBILITY
-from sugar_network.toolkit import http, util, lsb_release, pipe, exception
+from sugar_network.toolkit.spec import parse_version
+from sugar_network.toolkit import http, lsb_release, pipe, exception
 
 sys.path.insert(0, join(dirname(__file__), '..', 'lib', 'zeroinstall'))
 
@@ -253,7 +254,7 @@ class _Feed(model.ZeroInstallFeed):
         top_package = packages[0]
 
         impl = _Implementation(self, self.context, None)
-        impl.version = util.parse_version(top_package['version'])
+        impl.version = parse_version(top_package['version'])
         impl.released = 0
         impl.arch = '*-%s' % top_package['arch']
         impl.upstream_stability = model.stability_levels['packaged']
@@ -266,7 +267,7 @@ class _Feed(model.ZeroInstallFeed):
         impl_id = release['guid']
 
         impl = _Implementation(self, impl_id, None)
-        impl.version = util.parse_version(release['version'])
+        impl.version = parse_version(release['version'])
         impl.released = 0
         impl.arch = release['arch']
         impl.upstream_stability = model.stability_levels[release['stability']]
@@ -291,7 +292,7 @@ class _Feed(model.ZeroInstallFeed):
     def implement_sugar(self, sugar_version):
         impl_id = 'sugar-%s' % sugar_version
         impl = _Implementation(self, impl_id, None)
-        impl.version = util.parse_version(sugar_version)
+        impl.version = parse_version(sugar_version)
         impl.released = 0
         impl.arch = '*-*'
         impl.upstream_stability = model.stability_levels['packaged']
@@ -317,8 +318,8 @@ class _Dependency(model.InterfaceDependency):
 
         for not_before, before in data.get('restrictions') or []:
             restriction = model.VersionRangeRestriction(
-                    not_before=util.parse_version(not_before),
-                    before=util.parse_version(before))
+                    not_before=parse_version(not_before),
+                    before=parse_version(before))
             self.restrictions.append(restriction)
 
     @property
diff --git a/sugar_network/toolkit/spec.py b/sugar_network/toolkit/spec.py
index d9c617d..91afc0d 100644
--- a/sugar_network/toolkit/spec.py
+++ b/sugar_network/toolkit/spec.py
@@ -20,12 +20,10 @@ from os.path import join, exists, dirname
 from ConfigParser import ConfigParser
 
 from sugar_network.toolkit.licenses import GOOD_LICENSES
-from sugar_network.toolkit import util, enforce
+from sugar_network.toolkit import exception, enforce
 
 
-_LIST_SEPARATOR = ';'
-
-_POLICY_URL = 'http://wiki.sugarlabs.org/go/Sugar_Network/Policy'
+EMPTY_LICENSE = 'License is not specified'
 
 _FIELDS = {
         # name: (required, typecast)
@@ -43,12 +41,111 @@ _FIELDS = {
         }
 _ARCHES = ['all', 'any']
 _STABILITIES = ('insecure', 'buggy', 'developer', 'testing', 'stable')
+_POLICY_URL = 'http://wiki.sugarlabs.org/go/Sugar_Network/Policy'
+_LIST_SEPARATOR = ';'
 
 _RESTRICTION_RE = re.compile('(>=|<|=)\\s*([0-9.]+)')
 
+_VERSION_RE = re.compile('-([a-z]*)')
+_VERSION_MOD_TO_VALUE = {
+        'pre': -2,
+        'rc': -1,
+        '': 0,
+        'r': 1,
+        'post': 2,
+        }
+_VERSION_VALUE_TO_MOD = {}
+
 _logger = logging.getLogger('sweets-recipe')
 
 
+def parse_version(version_string):
+    """Convert a version string to an internal representation.
+
+    The parsed format can be compared quickly using the standard Python
+    functions. Adapted Zero Install version.
+
+    :param version_string:
+        version in format supported by 0install
+    :returns:
+        array of arrays of integers
+
+    """
+    if version_string is None:
+        return None
+
+    parts = _VERSION_RE.split(version_string)
+    if parts[-1] == '':
+        del parts[-1]  # Ends with a modifier
+    else:
+        parts.append('')
+    enforce(parts, ValueError, 'Empty version string')
+
+    length = len(parts)
+    try:
+        for x in range(0, length, 2):
+            part = parts[x]
+            if part:
+                parts[x] = [int(i) for i in part.split('.')]
+            else:
+                parts[x] = []  # (because ''.split('.') == [''], not [])
+        for x in range(1, length, 2):
+            parts[x] = _VERSION_MOD_TO_VALUE[parts[x]]
+        return parts
+    except ValueError as error:
+        exception()
+        raise ValueError('Invalid version format in "%s": %s' %
+                (version_string, error))
+    except KeyError as error:
+        raise ValueError('Invalid version modifier in "%s": %s' %
+                (version_string, error))
+
+
+def format_version(version):
+    """Convert version to string representation.
+
+    If string value is passed, it will be parsed to procuduce
+    canonicalized string representation.
+
+    """
+    if version is None:
+        return None
+    if isinstance(version, basestring):
+        version = parse_version(version)
+
+    if not _VERSION_VALUE_TO_MOD:
+        for mod, value in _VERSION_MOD_TO_VALUE.items():
+            _VERSION_VALUE_TO_MOD[value] = mod
+
+    version = version[:]
+    length = len(version)
+
+    for x in range(0, length, 2):
+        version[x] = '.'.join([str(i) for i in version[x]])
+    for x in range(1, length, 2):
+        version[x] = '-' + _VERSION_VALUE_TO_MOD[version[x]]
+    if version[-1] == '-':
+        del version[-1]
+
+    return ''.join(version)
+
+
+def format_next_version(version):
+    """Convert incremented version to string representation.
+
+    Before convertation, the last version's rank will be incremented.
+    If string value is passed, it will be parsed to procuduce
+    canonicalized string representation.
+
+    """
+    if version is None:
+        return None
+    if isinstance(version, basestring):
+        version = parse_version(version)
+    version[-2][-1] += 1
+    return format_version(version)
+
+
 class Spec(object):
 
     def __init__(self, spec=None, root=None):
@@ -189,7 +286,7 @@ class Spec(object):
             if not self['icon'].lower().endswith('.svg'):
                 self._fields['icon'] = join('activity', self['icon'] + '.svg')
             if not self['license']:
-                self._fields['license'] = 'license is not specified'
+                self._fields['license'] = EMPTY_LICENSE
 
         for key, (required, __) in _FIELDS.items():
             enforce(not required or key in self._fields,
@@ -200,8 +297,7 @@ class Spec(object):
 
         if not self['description']:
             self._fields['description'] = self['summary']
-        self._fields['version'] = \
-                util.format_version(util.parse_version(self['version']))
+        self._fields['version'] = format_version(self['version'])
 
         if not self.archives:
             self.archives.append(_Archive(self._config, 'DEFAULT'))
@@ -303,6 +399,21 @@ class _Command(dict):
         self.requires = {}
 
 
+class _Dependency(dict):
+
+    def versions_range(self):
+        for not_before, before in self.get('restrictions') or []:
+            i = parse_version(not_before)[0]
+            yield format_version([i, 0])
+            end = parse_version(before)[0]
+            i = i[:min(len(i), len(end))]
+            while True:
+                i[-1] += 1
+                if i >= end:
+                    break
+                yield format_version([i, 0])
+
+
 def _parse_bindings(text):
     result = set()
 
@@ -330,7 +441,7 @@ def _parse_requires(requires):
     result = {}
 
     for dep_str in _parse_list(requires):
-        dep = {}
+        dep = _Dependency()
 
         if dep_str.startswith('[') and dep_str.endswith(']'):
             dep_str = dep_str[1:-1]
@@ -348,17 +459,18 @@ def _parse_requires(requires):
 
         not_before = None
         before = None
-
         while len(parts) >= 3:
-            if parts[0] == '>=':
-                not_before = util.format_version(util.parse_version(parts[1]))
-            elif parts[0] == '<':
-                before = util.format_version(util.parse_version(parts[1]))
+            if parts[0] == '<':
+                before = format_version(parts[1])
+            elif parts[0] == '<=':
+                before = format_next_version(parts[1])
+            elif parts[0] == '>':
+                not_before = format_next_version(parts[1])
+            elif parts[0] == '>=':
+                not_before = format_version(parts[1])
             elif parts[0] == '=':
-                not_before = util.format_version(util.parse_version(parts[1]))
-                before = util.parse_version(parts[1])
-                before[-2][-1] += 1
-                before = util.format_version(before)
+                not_before = format_version(parts[1])
+                before = format_next_version(parts[1])
             del parts[:3]
 
         enforce(not parts or not parts[0].strip(),
diff --git a/sugar_network/toolkit/util.py b/sugar_network/toolkit/util.py
index 9620efa..8fc26cf 100644
--- a/sugar_network/toolkit/util.py
+++ b/sugar_network/toolkit/util.py
@@ -16,7 +16,6 @@
 """Swiss knife module."""
 
 import os
-import re
 import json
 import logging
 import hashlib
@@ -25,19 +24,9 @@ import collections
 from os.path import exists, join, islink, isdir, dirname, basename, abspath
 from os.path import lexists, isfile
 
-from sugar_network.toolkit import BUFFER_SIZE, cachedir, exception, enforce
+from sugar_network.toolkit import BUFFER_SIZE, cachedir, enforce
 
 
-_VERSION_RE = re.compile('-([a-z]*)')
-_VERSION_MOD_TO_VALUE = {
-        'pre': -2,
-        'rc': -1,
-        '': 0,
-        'post': 1,
-        'r': 1,
-        }
-_VERSION_VALUE_TO_MOD = {}
-
 _logger = logging.getLogger('toolkit.util')
 
 
@@ -100,70 +89,6 @@ def iter_file(*path):
             yield chunk
 
 
-def parse_version(version_string):
-    """Convert a version string to an internal representation.
-
-    The parsed format can be compared quickly using the standard Python
-    functions. Adapted Zero Install version.
-
-    :param version_string:
-        version in format supported by 0install
-    :returns:
-        array of arrays of integers
-
-    """
-    if version_string is None:
-        return None
-
-    parts = _VERSION_RE.split(version_string)
-    if parts[-1] == '':
-        del parts[-1]  # Ends with a modifier
-    else:
-        parts.append('')
-    enforce(parts, ValueError, 'Empty version string')
-
-    length = len(parts)
-    try:
-        for x in range(0, length, 2):
-            part = parts[x]
-            if part:
-                parts[x] = [int(i or '0') for i in part.split('.')]
-            else:
-                parts[x] = []  # (because ''.split('.') == [''], not [])
-        for x in range(1, length, 2):
-            parts[x] = _VERSION_MOD_TO_VALUE[parts[x]]
-        return parts
-    except ValueError as error:
-        exception()
-        raise RuntimeError('Invalid version format in "%s": %s' %
-                (version_string, error))
-    except KeyError as error:
-        raise RuntimeError('Invalid version modifier in "%s": %s' %
-                (version_string, error))
-
-
-def format_version(version):
-    """Convert internal version representation back to string."""
-    if version is None:
-        return None
-
-    if not _VERSION_VALUE_TO_MOD:
-        for mod, value in _VERSION_MOD_TO_VALUE.items():
-            _VERSION_VALUE_TO_MOD[value] = mod
-
-    version = version[:]
-    length = len(version)
-
-    for x in range(0, length, 2):
-        version[x] = '.'.join([str(i) for i in version[x]])
-    for x in range(1, length, 2):
-        version[x] = '-' + _VERSION_VALUE_TO_MOD[version[x]]
-    if version[-1] == '-':
-        del version[-1]
-
-    return ''.join(version)
-
-
 def readline(stream, limit=None):
     line = bytearray()
     while limit is None or len(line) < limit:
diff --git a/tests/units/toolkit/spec.py b/tests/units/toolkit/spec.py
index 891dbe8..1cdf36e 100755
--- a/tests/units/toolkit/spec.py
+++ b/tests/units/toolkit/spec.py
@@ -10,6 +10,29 @@ from sugar_network.toolkit import spec
 
 class SpecTest(tests.Test):
 
+    def test_Dependency(self):
+        self.assertEqual(
+                [],
+                [i for i in spec._Dependency().versions_range()])
+        self.assertEqual(
+                [],
+                [i for i in spec._Dependency({'restrictions': []}).versions_range()])
+        self.assertEqual(
+                ['1'],
+                [i for i in spec._Dependency({'restrictions': [('1', '2')]}).versions_range()])
+        self.assertEqual(
+                ['1.2'],
+                [i for i in spec._Dependency({'restrictions': [('1.2', '1.2.999')]}).versions_range()])
+        self.assertEqual(
+                ['1.2', '1.3'],
+                [i for i in spec._Dependency({'restrictions': [('1.2', '1.4')]}).versions_range()])
+        self.assertEqual(
+                ['1.2.3', '1.3'],
+                [i for i in spec._Dependency({'restrictions': [('1.2.3', '1.4')]}).versions_range()])
+        self.assertEqual(
+                ['1.2', '1.3', '1.4'],
+                [i for i in spec._Dependency({'restrictions': [('1.2', '1.4.5')]}).versions_range()])
+
     def test_parse_requires(self):
         self.assertEqual(
                 {'a': {}, 'b': {}, 'c': {}},
@@ -97,6 +120,54 @@ class SpecTest(tests.Test):
                     },
                 recipe.requires)
 
+    def testVersions(self):
+
+        def pv(v):
+            parsed = spec.parse_version(v)
+            self.assertEqual(v, spec.format_version(parsed))
+            return parsed
+
+        assert pv('1.0') > pv('0.9')
+        assert pv('1.0') > pv('1')
+        assert pv('1.0') == pv('1.0')
+        assert pv('0.9.9') < pv('1.0')
+        assert pv('10') > pv('2')
+
+        self.assertRaises(ValueError, spec.parse_version, '.')
+        self.assertRaises(ValueError, spec.parse_version, 'hello')
+        self.assertRaises(ValueError, spec.parse_version, '2./1')
+        self.assertRaises(ValueError, spec.parse_version, '.1')
+        self.assertRaises(ValueError, spec.parse_version, '')
+
+        # Check parsing
+        self.assertEqual([[1], 0], pv('1'))
+        self.assertEqual([[1,0], 0], pv('1.0'))
+        self.assertEqual([[1,0], -2, [5], 0], pv('1.0-pre5'))
+        self.assertEqual([[1,0], -1, [5], 0], pv('1.0-rc5'))
+        self.assertEqual([[1,0], 0, [5], 0], pv('1.0-5'))
+        self.assertEqual([[1,0], 1, [5], 0], pv('1.0-r5'))
+        self.assertEqual([[1,0], 2, [5], 0], pv('1.0-post5'))
+        self.assertEqual([[1,0], 1], pv('1.0-r'))
+        self.assertEqual([[1,0], 2], pv('1.0-post'))
+        self.assertEqual([[1], -1, [2,0], -2, [2], 1], pv('1-rc2.0-pre2-r'))
+        self.assertEqual([[1], -1, [2,0], -2, [2], 2], pv('1-rc2.0-pre2-post'))
+        self.assertEqual([[1], -1, [2,0], -2, [], 1], pv('1-rc2.0-pre-r'))
+        self.assertEqual([[1], -1, [2,0], -2, [], 2], pv('1-rc2.0-pre-post'))
+
+        assert pv('1.0-0') > pv('1.0')
+        assert pv('1.0-1') > pv('1.0-0')
+        assert pv('1.0-0') < pv('1.0-1')
+
+        assert pv('1.0-pre99') > pv('1.0-pre1')
+        assert pv('1.0-pre99') < pv('1.0-rc1')
+        assert pv('1.0-rc1') < pv('1.0')
+        assert pv('1.0') < pv('1.0-0')
+        assert pv('1.0-0') < pv('1.0-r')
+        assert pv('1.0-r') < pv('1.0-post')
+        assert pv('2.1.9-pre-1') > pv('2.1.9-pre')
+
+        assert pv('2-r999') < pv('3-pre1')
+
 
 if __name__ == '__main__':
     tests.main()