completing marker functionality

glance/registry/db/api.py

@@ -175,6 +175,12 @@ def image_get_all_public(context, filters=None, marker=None, limit=None):
         query = query.filter(getattr(models.Image, k) == v)
 
     if marker != None:
+        # images returned should be created before the image defined by marker
+        marker_created_at = image_get(context, marker, session).created_at
+        query = query.filter(models.Image.created_at < marker_created_at)
+
+        # ensure marker is respected if there are multiple images with the same
+        # created_at timestamp. This only works if also sorted by id
         query = query.filter(models.Image.id < marker)
 
     if limit != None:

tests/functional/test_curl_api.py

@@ -1045,6 +1045,7 @@ class TestCurlApi(functional.FunctionalTest):
         status_line = lines[0]
 
         self.assertEqual("HTTP/1.1 201 Created", status_line)
+
         cmd = ("curl -i -X POST "
                "-H 'Expect: ' "  # Necessary otherwise sends 100 Continue
                "-H 'X-Image-Meta-Name: Image3' "
@@ -1066,26 +1067,26 @@ class TestCurlApi(functional.FunctionalTest):
         exitcode, out, err = execute(cmd)
 
         self.assertEqual(0, exitcode)
-        images = json.loads(out.strip())
+        images = json.loads(out.strip())['images']
 
-        self.assertEqual(len(images['images']), 2)
+        self.assertEqual(len(images), 2)
-        for image in images['images']:
+        self.assertEqual(int(images[0]['id']), 3)
-            self.assertTrue(image['id'] >= 2)
+        self.assertEqual(int(images[1]['id']), 2)
 
-        # 3. GET /images with marker 2
+        # 3. GET /images with marker
         # Verify only two images were returned
         cmd = "curl http://0.0.0.0:%d/v1/images?marker=3" % api_port
 
         exitcode, out, err = execute(cmd)
 
         self.assertEqual(0, exitcode)
-        images = json.loads(out.strip())
+        images = json.loads(out.strip())['images']
 
-        self.assertEqual(len(images['images']), 2)
+        self.assertEqual(len(images), 2)
-        for image in images['images']:
+        self.assertEqual(int(images[0]['id']), 2)
-            self.assertTrue(image['id'] <= 2)
+        self.assertEqual(int(images[1]['id']), 1)
 
-        # 4. GET /images with marker 1 and limit of 1
+        # 4. GET /images with marker and limit
         # Verify only one image was returned with the correct id
         cmd = ("curl 'http://0.0.0.0:%d/v1/images?"
                "limit=1&marker=2'" % api_port)
@@ -1093,12 +1094,12 @@ class TestCurlApi(functional.FunctionalTest):
         exitcode, out, err = execute(cmd)
 
         self.assertEqual(0, exitcode)
-        images = json.loads(out.strip())
+        images = json.loads(out.strip())['images']
 
-        self.assertEqual(len(images['images']), 1)
+        self.assertEqual(len(images), 1)
-        self.assertEqual(images['images'][0]['id'], 1)
+        self.assertEqual(int(images[0]['id']), 1)
 
-        # 5. GET /images/detail with marker 1 and limit of 1
+        # 5. GET /images/detail with marker and limit
         # Verify only one image was returned with the correct id
         cmd = ("curl 'http://0.0.0.0:%d/v1/images/detail?"
                "limit=1&marker=3'" % api_port)
@@ -1106,7 +1107,7 @@ class TestCurlApi(functional.FunctionalTest):
         exitcode, out, err = execute(cmd)
 
         self.assertEqual(0, exitcode)
-        images = json.loads(out.strip())
+        images = json.loads(out.strip())['images']
 
-        self.assertEqual(len(images['images']), 1)
+        self.assertEqual(len(images), 1)
-        self.assertEqual(images['images'][0]['id'], 2)
+        self.assertEqual(int(images[0]['id']), 2)

tests/stubs.py

@@ -19,6 +19,7 @@
 
 import datetime
 import httplib
+import operator
 import os
 import shutil
 import StringIO
@@ -322,8 +323,10 @@ def stub_out_registry_db_image_api(stubs):
             values['deleted'] = False
             values['properties'] = values.get('properties', {})
             values['location'] = values.get('location')
-            values['created_at'] = datetime.datetime.utcnow()
+
-            values['updated_at'] = datetime.datetime.utcnow()
+            now = datetime.datetime.utcnow()
+            values['created_at'] = values.get('created_at', now)
+            values['updated_at'] = values.get('updated_at', now)
             values['deleted_at'] = None
 
             props = []
@@ -334,8 +337,8 @@ def stub_out_registry_db_image_api(stubs):
                     p['name'] = k
                     p['value'] = v
                     p['deleted'] = False
-                    p['created_at'] = datetime.datetime.utcnow()
+                    p['created_at'] = now
-                    p['updated_at'] = datetime.datetime.utcnow()
+                    p['updated_at'] = now
                     p['deleted_at'] = None
                     props.append(p)
 
@@ -412,7 +415,18 @@ def stub_out_registry_db_image_api(stubs):
             for k, v in filters.items():
                 images = [f for f in images if f[k] == v]
 
-            images = sorted(images, key=lambda i: i['created_at'])
+            def image_cmp(x, y):
+                if x['created_at'] > y['created_at']:
+                    return 1
+                elif x['created_at'] == y['created_at']:
+                    if x['id'] > y['id']:
+                        return 1
+                    else:
+                        return -1
+                else:
+                    return -1
+
+            images = sorted(images, cmp=image_cmp)
             images.reverse()
 
             if marker == None:

tests/unit/test_api.py

@@ -15,6 +15,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import datetime
 import hashlib
 import httplib
 import os
@@ -93,6 +94,10 @@ class TestRegistryAPI(unittest.TestCase):
         public images that conforms to a marker query param
 
         """
+
+        time1 = datetime.datetime.utcnow() + datetime.timedelta(seconds=5)
+        time2 = datetime.datetime.utcnow()
+
         extra_fixture = {'id': 3,
                          'status': 'active',
                          'is_public': True,
@@ -100,7 +105,8 @@ class TestRegistryAPI(unittest.TestCase):
                          'container_format': 'ovf',
                          'name': 'new name! #123',
                          'size': 19,
-                         'checksum': None}
+                         'checksum': None,
+                         'created_at': time1}
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
@@ -111,7 +117,20 @@ class TestRegistryAPI(unittest.TestCase):
                          'container_format': 'ovf',
                          'name': 'new name! #123',
                          'size': 20,
-                         'checksum': None}
+                         'checksum': None,
+                         'created_at': time1}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 5,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None,
+                         'created_at': time2}
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
@@ -121,11 +140,12 @@ class TestRegistryAPI(unittest.TestCase):
         self.assertEquals(res.status_int, 200)
 
         images = res_dict['images']
-        self.assertEquals(len(images), 2)
+        # should be sorted by created_at desc, id desc
-
+        # page should start after marker 4
-        # expect list to be sorted by created_at desc
+        self.assertEquals(len(images), 3)
-        for image in images:
+        self.assertEquals(int(images[0]['id']), 3)
-            self.assertTrue(int(image['id']) < 4)
+        self.assertEquals(int(images[1]['id']), 5)
+        self.assertEquals(int(images[2]['id']), 2)
 
     def test_get_index_limit(self):
         """Tests that the /images registry API returns list of