Cleanup and test Location, and add the ability to specify a revision

common/lib/keystore/__init__.py

@@ -10,47 +10,117 @@ the following attributes:
     revision: What revision of the item this is
 """
 
+import re
+from .exceptions import InvalidLocationError
+
+URL_RE = re.compile("""
+    (?P<tag>[^:]+)://
+    (?P<org>[^/]+)/
+    (?P<course>[^/]+)/
+    (?P<category>[^/]+)/
+    (?P<name>[^/]+)
+    (/(?P<revision>[^/]+))?
+    """, re.VERBOSE)
+
 
 class Location(object):
-    ''' Encodes a location.
-        Can be:
-        * String (url)
-        * Tuple
-        * Dictionary
+    '''
+    Encodes a location.
+
+    Locations representations of URLs of the
+    form {tag}://{org}/{course}/{category}/{name}[/{revision}]
+
+    However, they can also be represented a dictionaries (specifying each component),
+    tuples or list (specified in order), or as strings of the url
     '''
     def __init__(self, location):
+        """
+        Create a new location that is a clone of the specifed one.
+
+        location - Can be any of the following types:
+            string: should be of the form {tag}://{org}/{course}/{category}/{name}[/{revision}]
+            list: should be of the form [tag, org, course, category, name, revision]
+            dict: should be of the form {
+                'tag': tag,
+                'org': org,
+                'course': course,
+                'category': category,
+                'name': name,
+                'revision': revision,
+            }
+            Location: another Location object
+
+        None of the components of a location may contain the '/' character
+        """
         self.update(location)
 
     def update(self, location):
+        """
+        Update this instance with data from another Location object.
+
+        location: can take the same forms as specified by `__init__`
+        """
+        self.tag = self.org = self.course = self.category = self.name = self.revision = None
+
         if isinstance(location, basestring):
-            self.tag = location.split('/')[0][:-1]
-            (self.org, self.course, self.category, self.name) = location.split('/')[2:]
+            match = URL_RE.match(location)
+            if match is None:
+                raise InvalidLocationError(location)
+            else:
+                self.update(match.groupdict())
         elif isinstance(location, list):
-            (self.tag, self.org, self.course, self.category, self.name) = location
+            if len(location) not in (5, 6):
+                raise InvalidLocationError(location)
+
+            (self.tag, self.org, self.course, self.category, self.name) = location[0:5]
+            self.revision = location[5] if len(location) == 6 else None
         elif isinstance(location, dict):
-            self.tag = location['tag']
-            self.org = location['org']
-            self.course = location['course']
-            self.category = location['category']
-            self.name = location['name']
+            try:
+                self.tag = location['tag']
+                self.org = location['org']
+                self.course = location['course']
+                self.category = location['category']
+                self.name = location['name']
+            except KeyError:
+                raise InvalidLocationError(location)
+            self.revision = location.get('revision')
         elif isinstance(location, Location):
             self.update(location.list())
+        else:
+            raise InvalidLocationError(location)
+
+        for val in self.list():
+            if val is not None and '/' in val:
+                raise InvalidLocationError(location)
+
+    def __str__(self):
+        return self.url()
 
     def url(self):
-        return "{tag}://{org}/{course}/{category}/{name}".format(**self.dict())
+        """
+        Return a string containing the URL for this location
+        """
+        url = "{tag}://{org}/{course}/{category}/{name}".format(**self.dict())
+        if self.revision:
+            url += "/" + self.revision
+        return url
 
     def list(self):
-        return [self.tag, self.org, self.course, self.category, self.name]
+        """
+        Return a list representing this location
+        """
+        return [self.tag, self.org, self.course, self.category, self.name, self.revision]
 
     def dict(self):
+        """
+        Return a dictionary representing this location
+        """
         return {'tag': self.tag,
                 'org': self.org,
                 'course': self.course,
                 'category': self.category,
-                'name': self.name}
-
-    def to_json(self):
-        return self.dict()
+                'name': self.name,
+                'revision': self.revision}
 
 
 class KeyStore(object):

common/lib/keystore/exceptions.py

@@ -9,3 +9,6 @@ class ItemNotFoundError(Exception):
 
 class InsufficientSpecificationError(Exception):
     pass
+
+class InvalidLocationError(Exception):
+    pass

common/lib/keystore/test/test_location.py

+from nose.tools import assert_equals, assert_raises
+from keystore import Location
+from keystore.exceptions import InvalidLocationError
+
+
+def check_string_roundtrip(url):
+    assert_equals(url, Location(url).url())
+    assert_equals(url, str(Location(url)))
+
+
+def test_string_roundtrip():
+    check_string_roundtrip("tag://org/course/category/name")
+    check_string_roundtrip("tag://org/course/category/name/revision")
+    check_string_roundtrip("tag://org/course/category/name with spaces/revision")
+
+
+def test_dict():
+    input_dict = {
+        'tag': 'tag',
+        'course': 'course',
+        'category': 'category',
+        'name': 'name',
+        'org': 'org'
+    }
+    assert_equals("tag://org/course/category/name", Location(input_dict).url())
+    assert_equals(dict(revision=None, **input_dict), Location(input_dict).dict())
+
+    input_dict['revision'] = 'revision'
+    assert_equals("tag://org/course/category/name/revision", Location(input_dict).url())
+    assert_equals(input_dict, Location(input_dict).dict())
+
+
+def test_list():
+    input_list = ['tag', 'org', 'course', 'category', 'name']
+    assert_equals("tag://org/course/category/name", Location(input_list).url())
+    assert_equals(input_list + [None], Location(input_list).list())
+
+    input_list.append('revision')
+    assert_equals("tag://org/course/category/name/revision", Location(input_list).url())
+    assert_equals(input_list, Location(input_list).list())
+
+
+def test_location():
+    input_list = ['tag', 'org', 'course', 'category', 'name']
+    assert_equals("tag://org/course/category/name", Location(Location(input_list)).url())
+
+
+def test_invalid_locations():
+    assert_raises(InvalidLocationError, Location, "foo")
+    assert_raises(InvalidLocationError, Location, ["foo", "bar"])
+    assert_raises(InvalidLocationError, Location, ["foo", "bar", "baz", "blat", "foo/bar"])
+    assert_raises(InvalidLocationError, Location, None)