Merge lp:~rackspace-titan/glance/api-results-filtering into lp:~hudson-openstack/glance/trunk

[Brian Waldon]

Adding support for api query filtering
- equality testing on select attributes: name, status, container_format, disk_format
- relative comparison of size attribute with size_min, size_max
- equality testing on user-defined properties (preface property name with "property-" in query)

[Brian Waldon (bcwaldon)]

I could use some help in registry/db/api.py with the sqlalchemy code. See the TODO

[Mark Washenberger (markwash)]

162 + #TODO(bcwaldon): use an actual sqlalchemy query to accomplish this
163 + def prop_filter(key, value):
164 + def func(image):
165 + for prop in image.properties:
166 + if prop.deleted == False and \
167 + prop.name == key and \
168 + prop.value == value:
169 + return True
170 + return False
171 + return func
172 +
173 + for (k, v) in filters.items():
174 + if k.startswith('property-'):
175 + _k = k[9:]
176 + images = filter(prop_filter(_k, v), images)

I should really test this before suggesting it, but:

for (k, v) in filters.items():
 if k.startswith('property-'):
  _k = k[9:]
  query.filter(models.Images.properties.any(and_(
   models.ImageProperty.name == _k,
   models.ImageProperty.value == v)))

Also maybe

for (k, v) in filters.items():
 if k.startswith('property-'):
  _k = k[9:]
  query.filter(models.Images.properties.any(name=_k, value=v))

At the end I think you can then just return query.all()

[lp:~rackspace-titan/glance/api-results-filtering]

136. By Brian Waldon on 2011-05-16

consolidating image_get_all_public and image_get_filtered in registry db api

137. By Brian Waldon on 2011-05-16

making registry db api filters more structured; adding in a bit of sqlalchemy code to filter image properties more efficiently

[Brian Waldon (bcwaldon)]

Thanks, Mark. Your second suggestion ended up working.

I did some refactoring of where the filter query params are parsed, as well.

[Dan Prince (dan-prince)]

I'm probably gonna get shot up but here it goes... (metadata vs. properties)
--

I think you've added an inconsistency in the HTTP API's by using 'property-'. While we call them properties internally we usually refer to them as 'meta' or metadata in the external interfaces to glance.

The bin/glance utility for example uses the following (snips):

    update Updates an image's metadata in Glance
    clear Removes all images and metadata from Glance

Additionally we update metadata(aka properties) with HTTP headers that look like this 'X-Image-Meta-Is-Public: True'.

Given these examples how would you feel about using 'meta-' instead of 'property-' for consistency?

---

Also a minor nit with the import statements in glance/registry/client.py. Most of those import statements can now be removed since they are now defined and used only within the BaseClient class.

review: Needs Fixing

[Brian Waldon (bcwaldon)]

>
> I'm probably gonna get shot up but here it goes... (metadata vs. properties)
> --
>
> I think you've added an inconsistency in the HTTP API's by using 'property-'.
> While we call them properties internally we usually refer to them as 'meta' or
> metadata in the external interfaces to glance.
>
> The bin/glance utility for example uses the following (snips):
>
> update Updates an image's metadata in Glance
> clear Removes all images and metadata from Glance
>
> Additionally we update metadata(aka properties) with HTTP headers that look
> like this 'X-Image-Meta-Is-Public: True'.
>
> Given these examples how would you feel about using 'meta-' instead of
> 'property-' for consistency?
>

I definitely understand why you would be concerned, but I think you may have overlooked one thing. I only use "property-" for user-defined image metadata, which you provide in the headers with "X-Image-Meta-Property..." The image data structure even uses a dictionary called "properties" to store the values I am referring to. The "X-Image-Meta-" header is reserved for actual image attributes, such as "name" and "status".

> Also a minor nit with the import statements in glance/registry/client.py. Most
> of those import statements can now be removed since they are now defined and
> used only within the BaseClient class.

Good catch. Removed those I found to be unnecessary.

[lp:~rackspace-titan/glance/api-results-filtering]

138. By Brian Waldon on 2011-05-16

removing some unnecessary imports

[Dan Prince (dan-prince)]

Sure. So why not make it 'meta-property-' for consistency then?

I realize this makes the URL for filtering a bit longer but I really like the consistency. If Glance could make up its mind and just go for either 'metadata' and/or 'properties' this sort of thing might be easier.

[Brian Waldon (bcwaldon)]

> Sure. So why not make it 'meta-property-' for consistency then?
>
> I realize this makes the URL for filtering a bit longer but I really like the
> consistency. If Glance could make up its mind and just go for either
> 'metadata' and/or 'properties' this sort of thing might be easier.

I'm still a bit confused. If I were to make it 'meta-property-' then I would have to make the other filters 'meta-name', 'meta-status', and so on. I don't think it is as confusing as you are making it out to be.

[Dan Prince (dan-prince)]

Sure. I no confusion. Just consistency concerns. Thanks for the clarification. I'm good with this. Good work.

review: Approve

[Jay Pipes (jaypipes)]

> Sure. So why not make it 'meta-property-' for consistency then?
>
> I realize this makes the URL for filtering a bit longer but I really like the
> consistency. If Glance could make up its mind and just go for either
> 'metadata' and/or 'properties' this sort of thing might be easier.

It's not about making up our minds. :) It's because they are different things. I would have preferred not using the term metadata at all, but that's what is used for "base image attributes". "properties" are the custom key-value attribute pairs attached to the image.

-jay

[Jay Pipes (jaypipes)]

Hey Brian,

Excellent work overall. Just one thing to note, though...

34 + """Return a dictionary of query param filters from the request
35 +
36 + :param req: the Request object coming from the wsgi layer

In Glance, much of the code uses the docstring style:

"""
Return a dictionary of query param filters from the request

:param req: the Request object coming from the wsgi layer
...
"""

Feel free to use it too if you prefer it. ;) I personally can't stand the Nova style, as I think it makes things harder to read.

-jay

review: Approve

[Brian Waldon (bcwaldon)]

> Hey Brian,
>
> Excellent work overall. Just one thing to note, though...
>
> 34 + """Return a dictionary of query param filters from the request
> 35 +
> 36 + :param req: the Request object coming from the wsgi layer
>
> In Glance, much of the code uses the docstring style:
>
> """
> Return a dictionary of query param filters from the request
>
> :param req: the Request object coming from the wsgi layer
> ...
> """
>
> Feel free to use it too if you prefer it. ;) I personally can't stand the Nova
> style, as I think it makes things harder to read.
>
> -jay

Made the change. I do prefer your style, and typically keep with whatever style is already established in the file. I guess I was in nova-mode.

[lp:~rackspace-titan/glance/api-results-filtering]

139. By Brian Waldon on 2011-05-17

docstring fix

=== modified file 'glance/api/v1/images.py'
--- glance/api/v1/images.py	2011-05-11 23:03:51 +0000
+++ glance/api/v1/images.py	2011-05-17 13:32:06 +0000
@@ -42,6 +42,9 @@
 
 logger = logging.getLogger('glance.api.v1.images')
 
+SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format',
+                     'size_min', 'size_max']
+
 
 class Controller(wsgi.Controller):
 
@@ -89,7 +92,8 @@
                  'size': <SIZE>}, ...
             ]}
         """
-        images = registry.get_images_list(self.options)
+        filters = self._get_filters(req)
+        images = registry.get_images_list(self.options, filters)
         return dict(images=images)
 
     def detail(self, req):
@@ -114,9 +118,24 @@
                  'properties': {'distro': 'Ubuntu 10.04 LTS', ...}}, ...
             ]}
         """
-        images = registry.get_images_detail(self.options)
+        filters = self._get_filters(req)
+        images = registry.get_images_detail(self.options, filters)
         return dict(images=images)
 
+    def _get_filters(self, req):
+        """
+        Return a dictionary of query param filters from the request
+
+        :param req: the Request object coming from the wsgi layer
+        :retval a dict of key/value filters
+        """
+        filters = {}
+        for param in req.str_params:
+            if param in SUPPORTED_FILTERS or param.startswith('property-'):
+                filters[param] = req.str_params.get(param)
+
+        return filters
+
     def meta(self, req, id):
         """
         Returns metadata about an image in the HTTP headers of the

=== modified file 'glance/registry/__init__.py'
--- glance/registry/__init__.py	2011-03-29 14:27:24 +0000
+++ glance/registry/__init__.py	2011-05-17 13:32:06 +0000
@@ -32,14 +32,14 @@
     return client.RegistryClient(host, port)
 
 
-def get_images_list(options):
-    c = get_registry_client(options)
-    return c.get_images()
-
-
-def get_images_detail(options):
-    c = get_registry_client(options)
-    return c.get_images_detailed()
+def get_images_list(options, filters):
+    c = get_registry_client(options)
+    return c.get_images(filters)
+
+
+def get_images_detail(options, filters):
+    c = get_registry_client(options)
+    return c.get_images_detailed(filters)
 
 
 def get_image_metadata(options, image_id):

=== modified file 'glance/registry/client.py'
--- glance/registry/client.py	2011-03-29 14:27:24 +0000
+++ glance/registry/client.py	2011-05-17 13:32:06 +0000
@@ -20,14 +20,9 @@
 the Glance Registry API
 """
 
-import httplib
 import json
-import logging
-import urlparse
-import socket
-import sys
+import urllib
 
-from glance.common import exception
 from glance.client import BaseClient
 
 
@@ -49,28 +44,34 @@
         port = port or self.DEFAULT_PORT
         super(RegistryClient, self).__init__(host, port, use_ssl)
 
-    def get_images(self):
+    def get_images(self, filters=None):
         """
         Returns a list of image id/name mappings from Registry
         """
-        res = self.do_request("GET", "/images")
+        if filters != None:
+            action = "/images?%s" % urllib.urlencode(filters)
+        else:
+            action = "/images"
+
+        res = self.do_request("GET", action)
         data = json.loads(res.read())['images']
         return data
 
-    def get_images_detailed(self):
+    def get_images_detailed(self, filters=None):
         """
         Returns a list of detailed image data mappings from Registry
         """
-        res = self.do_request("GET", "/images/detail")
+        if filters != None:
+            action = "/images/detail?%s" % urllib.urlencode(filters)
+        else:
+            action = "/images/detail"
+
+        res = self.do_request("GET", action)
         data = json.loads(res.read())['images']
         return data
 
     def get_image(self, image_id):
-        """
-        Returns a mapping of image metadata from Registry
-
-        :raises exception.NotFound if image is not in registry
-        """
+        """Returns a mapping of image metadata from Registry"""
         res = self.do_request("GET", "/images/%s" % image_id)
         data = json.loads(res.read())['image']
         return data

=== modified file 'glance/registry/db/api.py'
--- glance/registry/db/api.py	2011-05-04 15:03:28 +0000
+++ glance/registry/db/api.py	2011-05-17 13:32:06 +0000
@@ -139,15 +139,39 @@
         raise exception.NotFound("No image found with ID %s" % image_id)
 
 
-def image_get_all_public(context):
-    """Get all public images."""
+def image_get_all_public(context, filters=None):
+    """Get all public images that match zero or more filters.
+
+    :param filters: dict of filter keys and values. If a 'properties'
+                    key is present, it is treated as a dict of key/value
+                    filters on the image properties attribute
+
+    """
+    if filters == None:
+        filters = {}
+
     session = get_session()
-    return session.query(models.Image).\
+    query = session.query(models.Image).\
                    options(joinedload(models.Image.properties)).\
                    filter_by(deleted=_deleted(context)).\
                    filter_by(is_public=True).\
-                   filter(models.Image.status != 'killed').\
-                   all()
+                   filter(models.Image.status != 'killed')
+
+    if 'size_min' in filters:
+        query = query.filter(models.Image.size >= filters['size_min'])
+        del filters['size_min']
+
+    if 'size_max' in filters:
+        query = query.filter(models.Image.size <= filters['size_max'])
+        del filters['size_max']
+
+    for (k, v) in filters.pop('properties', {}).items():
+        query = query.filter(models.Image.properties.any(name=k, value=v))
+
+    for (k, v) in filters.items():
+        query = query.filter(getattr(models.Image, k) == v)
+
+    return query.all()
 
 
 def _drop_protected_attrs(model_class, values):

=== modified file 'glance/registry/server.py'
--- glance/registry/server.py	2011-04-07 19:07:36 +0000
+++ glance/registry/server.py	2011-05-17 13:32:06 +0000
@@ -36,6 +36,9 @@
                            'disk_format', 'container_format',
                            'checksum']
 
+SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format',
+                     'size_min', 'size_max']
+
 
 class Controller(wsgi.Controller):
     """Controller for the reference implementation registry server"""
@@ -45,7 +48,7 @@
         db_api.configure_db(options)
 
     def index(self, req):
-        """Return basic information for all public, non-deleted images
+        """Return a basic filtered list of public, non-deleted images
 
         :param req: the Request object coming from the wsgi layer
         :retval a mapping of the following form::
@@ -64,7 +67,8 @@
             }
 
         """
-        images = db_api.image_get_all_public(None)
+        images = db_api.image_get_all_public(None, self._get_filters(req))
+
         results = []
         for image in images:
             result = {}
@@ -74,7 +78,7 @@
         return dict(images=results)
 
     def detail(self, req):
-        """Return detailed information for all public, non-deleted images
+        """Return a filtered list of public, non-deleted images in detail
 
         :param req: the Request object coming from the wsgi layer
         :retval a mapping of the following form::
@@ -85,10 +89,33 @@
         all image model fields.
 
         """
-        images = db_api.image_get_all_public(None)
+        images = db_api.image_get_all_public(None, self._get_filters(req))
+
         image_dicts = [make_image_dict(i) for i in images]
         return dict(images=image_dicts)
 
+    def _get_filters(self, req):
+        """Return a dictionary of query param filters from the request
+
+        :param req: the Request object coming from the wsgi layer
+        :retval a dict of key/value filters
+
+        """
+        filters = {}
+        properties = {}
+
+        for param in req.str_params:
+            if param in SUPPORTED_FILTERS:
+                filters[param] = req.str_params.get(param)
+            if param.startswith('property-'):
+                _param = param[9:]
+                properties[_param] = req.str_params.get(param)
+
+        if len(properties) > 0:
+            filters['properties'] = properties
+
+        return filters
+
     def show(self, req, id):
         """Return data about the given image id."""
         try:

=== modified file 'tests/functional/test_curl_api.py'
--- tests/functional/test_curl_api.py	2011-05-13 22:28:51 +0000
+++ tests/functional/test_curl_api.py	2011-05-17 13:32:06 +0000
@@ -787,3 +787,213 @@
                             "Could not find '%s' in '%s'" % (expected, out))
 
         self.stop_servers()
+
+    def test_filtered_images(self):
+        """
+        Set up three test images and ensure each query param filter works
+        """
+        self.cleanup()
+        self.start_servers()
+
+        api_port = self.api_port
+        registry_port = self.registry_port
+
+        # 0. GET /images
+        # Verify no public images
+        cmd = "curl http://0.0.0.0:%d/v1/images" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        self.assertEqual('{"images": []}', out.strip())
+
+        # 1. POST /images with three public images with various attributes
+        cmd = ("curl -i -X POST "
+               "-H 'Expect: ' "  # Necessary otherwise sends 100 Continue
+               "-H 'X-Image-Meta-Name: Image1' "
+               "-H 'X-Image-Meta-Status: active' "
+               "-H 'X-Image-Meta-Container-Format: ovf' "
+               "-H 'X-Image-Meta-Disk-Format: vdi' "
+               "-H 'X-Image-Meta-Size: 19' "
+               "-H 'X-Image-Meta-Is-Public: True' "
+               "-H 'X-Image-Meta-Property-pants: are on' "
+               "http://0.0.0.0:%d/v1/images") % api_port
+
+        exitcode, out, err = execute(cmd)
+        self.assertEqual(0, exitcode)
+
+        lines = out.split("\r\n")
+        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: My Image!' "
+               "-H 'X-Image-Meta-Status: active' "
+               "-H 'X-Image-Meta-Container-Format: ovf' "
+               "-H 'X-Image-Meta-Disk-Format: vhd' "
+               "-H 'X-Image-Meta-Size: 20' "
+               "-H 'X-Image-Meta-Is-Public: True' "
+               "-H 'X-Image-Meta-Property-pants: are on' "
+               "http://0.0.0.0:%d/v1/images") % api_port
+
+        exitcode, out, err = execute(cmd)
+        self.assertEqual(0, exitcode)
+
+        lines = out.split("\r\n")
+        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: My Image!' "
+               "-H 'X-Image-Meta-Status: saving' "
+               "-H 'X-Image-Meta-Container-Format: ami' "
+               "-H 'X-Image-Meta-Disk-Format: ami' "
+               "-H 'X-Image-Meta-Size: 21' "
+               "-H 'X-Image-Meta-Is-Public: True' "
+               "-H 'X-Image-Meta-Property-pants: are off' "
+               "http://0.0.0.0:%d/v1/images") % api_port
+
+        exitcode, out, err = execute(cmd)
+        self.assertEqual(0, exitcode)
+
+        lines = out.split("\r\n")
+        status_line = lines[0]
+
+        self.assertEqual("HTTP/1.1 201 Created", status_line)
+
+        # 2. GET /images
+        # Verify three public images
+        cmd = "curl http://0.0.0.0:%d/v1/images" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 3)
+
+        # 3. GET /images with name filter
+        # Verify correct images returned with name
+        cmd = "curl http://0.0.0.0:%d/v1/images?name=My%%20Image!" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 2)
+        for image in images["images"]:
+            self.assertEqual(image["name"], "My Image!")
+
+        # 4. GET /images with status filter
+        # Verify correct images returned with status
+        cmd = ("curl http://0.0.0.0:%d/v1/images/detail?status=queued"
+               % api_port)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 3)
+        for image in images["images"]:
+            self.assertEqual(image["status"], "queued")
+
+        cmd = ("curl http://0.0.0.0:%d/v1/images/detail?status=active"
+               % api_port)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 0)
+
+        # 5. GET /images with container_format filter
+        # Verify correct images returned with container_format
+        cmd = ("curl http://0.0.0.0:%d/v1/images?container_format=ovf"
+               % api_port)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 2)
+        for image in images["images"]:
+            self.assertEqual(image["container_format"], "ovf")
+
+        # 6. GET /images with disk_format filter
+        # Verify correct images returned with disk_format
+        cmd = ("curl http://0.0.0.0:%d/v1/images?disk_format=vdi"
+               % api_port)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 1)
+        for image in images["images"]:
+            self.assertEqual(image["disk_format"], "vdi")
+
+        # 7. GET /images with size_max filter
+        # Verify correct images returned with size <= expected
+        cmd = ("curl http://0.0.0.0:%d/v1/images?size_max=20"
+               % api_port)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 2)
+        for image in images["images"]:
+            self.assertTrue(image["size"] <= 20)
+
+        # 8. GET /images with size_min filter
+        # Verify correct images returned with size >= expected
+        cmd = ("curl http://0.0.0.0:%d/v1/images?size_min=20"
+               % api_port)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 2)
+        for image in images["images"]:
+            self.assertTrue(image["size"] >= 20)
+
+        # 9. GET /images with property filter
+        # Verify correct images returned with property
+        cmd = ("curl http://0.0.0.0:%d/v1/images/detail?"
+               "property-pants=are%%20on" % api_port)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 2)
+        for image in images["images"]:
+            self.assertEqual(image["properties"]["pants"], "are on")
+
+        # 10. GET /images with property filter and name filter
+        # Verify correct images returned with property and name
+        # Make sure you quote the url when using more than one param!
+        cmd = ("curl 'http://0.0.0.0:%d/v1/images/detail?"
+               "name=My%%20Image!&property-pants=are%%20on'" % api_port)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())
+
+        self.assertEqual(len(images["images"]), 1)
+        for image in images["images"]:
+            self.assertEqual(image["properties"]["pants"], "are on")
+            self.assertEqual(image["name"], "My Image!")

=== modified file 'tests/stubs.py'
--- tests/stubs.py	2011-05-11 23:03:51 +0000
+++ tests/stubs.py	2011-05-17 13:32:06 +0000
@@ -386,9 +386,32 @@
             else:
                 return images[0]
 
-        def image_get_all_public(self, _context, public=True):
-            return [f for f in self.images
-                    if f['is_public'] == public]
+        def image_get_all_public(self, _context, filters):
+            images = [f for f in self.images if f['is_public'] == True]
+
+            if 'size_min' in filters:
+                size_min = int(filters.pop('size_min'))
+                images = [f for f in images if int(f['size']) >= size_min]
+
+            if 'size_max' in filters:
+                size_max = int(filters.pop('size_max'))
+                images = [f for f in images if int(f['size']) <= size_max]
+
+            def _prop_filter(key, value):
+                def _func(image):
+                    for prop in image['properties']:
+                        if prop['name'] == key:
+                            return prop['value'] == value
+                    return False
+                return _func
+
+            for k, v in filters.pop('properties', {}).items():
+                images = filter(_prop_filter(k, v), images)
+
+            for k, v in filters.items():
+                images = [f for f in images if f[k] == v]
+
+            return images
 
     fake_datastore = FakeDatastore()
     stubs.Set(glance.registry.db.api, 'image_create',

=== modified file 'tests/unit/test_api.py'
--- tests/unit/test_api.py	2011-05-11 23:03:51 +0000
+++ tests/unit/test_api.py	2011-05-17 13:32:06 +0000
@@ -26,6 +26,7 @@
 
 from glance.api import v1 as server
 from glance.registry import server as rserver
+import glance.registry.db.api
 from tests import stubs
 
 VERBOSE = False
@@ -87,6 +88,50 @@
         for k, v in fixture.iteritems():
             self.assertEquals(v, images[0][k])
 
+    def test_get_index_filter_name(self):
+        """Tests that the /images registry API returns list of
+        public images that have a specific name. This is really a sanity
+        check, filtering is tested more in-depth using /images/detail
+
+        """
+        fixture = {'id': 2,
+                   'name': 'fake image #2',
+                   'size': 19,
+                   'checksum': None}
+
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images?name=new name! #123')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertEqual('new name! #123', image['name'])
+
     def test_get_details(self):
         """Tests that the /images/detail registry API returns
         a mapping containing a list of detailed image information
@@ -112,6 +157,324 @@
         for k, v in fixture.iteritems():
             self.assertEquals(v, images[0][k])
 
+    def test_get_details_filter_name(self):
+        """Tests that the /images/detail registry API returns list of
+        public images that have a specific name
+
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?name=new name! #123')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertEqual('new name! #123', image['name'])
+
+    def test_get_details_filter_status(self):
+        """Tests that the /images/detail registry API returns list of
+        public images that have a specific status
+
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'saving',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'fake image #3',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'fake image #4',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?status=saving')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 1)
+
+        for image in images:
+            self.assertEqual('saving', image['status'])
+
+    def test_get_details_filter_container_format(self):
+        """Tests that the /images/detail registry API returns list of
+        public images that have a specific container_format
+
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vdi',
+                         'container_format': 'ovf',
+                         'name': 'fake image #3',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'fake image #4',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?container_format=ovf')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertEqual('ovf', image['container_format'])
+
+    def test_get_details_filter_disk_format(self):
+        """Tests that the /images/detail registry API returns list of
+        public images that have a specific disk_format
+
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'fake image #3',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'fake image #4',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?disk_format=vhd')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertEqual('vhd', image['disk_format'])
+
+    def test_get_details_filter_size_min(self):
+        """Tests that the /images/detail registry API returns list of
+        public images that have a size greater than or equal to size_min
+
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'fake image #3',
+                         'size': 18,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'fake image #4',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?size_min=19')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertTrue(image['size'] >= 19)
+
+    def test_get_details_filter_size_max(self):
+        """Tests that the /images/detail registry API returns list of
+        public images that have a size less than or equal to size_max
+
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'fake image #3',
+                         'size': 18,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'fake image #4',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?size_max=19')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertTrue(image['size'] <= 19)
+
+    def test_get_details_filter_size_min_max(self):
+        """Tests that the /images/detail registry API returns list of
+        public images that have a size less than or equal to size_max
+        and greater than or equal to size_min
+
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'fake image #3',
+                         'size': 18,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'fake image #4',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 5,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'fake image #5',
+                         'size': 6,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?size_min=18&size_max=19')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertTrue(image['size'] <= 19 and image['size'] >= 18)
+
+    def test_get_details_filter_property(self):
+        """Tests that the /images/detail registry API returns list of
+        public images that have a specific custom property
+
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'fake image #3',
+                         'size': 19,
+                         'checksum': None,
+                         'properties': {'prop_123': 'v a'}}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'fake image #4',
+                         'size': 19,
+                         'checksum': None,
+                         'properties': {'prop_123': 'v b'}}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?property-prop_123=v%20a')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 1)
+
+        for image in images:
+            self.assertEqual('v a', image['properties']['prop_123'])
+
     def test_create_image(self):
         """Tests that the /images POST registry API creates the image"""
         fixture = {'name': 'fake public image',

=== modified file 'tests/unit/test_clients.py'
--- tests/unit/test_clients.py	2011-05-05 23:12:21 +0000
+++ tests/unit/test_clients.py	2011-05-17 13:32:06 +0000
@@ -24,8 +24,9 @@
 import webob
 
 from glance import client
+from glance.common import exception
+import glance.registry.db.api
 from glance.registry import client as rclient
-from glance.common import exception
 from tests import stubs
 
 
@@ -69,6 +70,26 @@
         for k, v in fixture.items():
             self.assertEquals(v, images[0][k])
 
+    def test_get_image_index_by_name(self):
+        """Test correct set of public, name-filtered image returned. This
+        is just a sanity check, we test the details call more in-depth."""
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images({'name': 'new name! #123'})
+        self.assertEquals(len(images), 1)
+
+        for image in images:
+            self.assertEquals('new name! #123', image['name'])
+
     def test_get_image_details(self):
         """Tests that the detailed info about public images returned"""
         fixture = {'id': 2,
@@ -87,6 +108,140 @@
         for k, v in fixture.items():
             self.assertEquals(v, images[0][k])
 
+    def test_get_image_details_by_name(self):
+        """Tests that a detailed call can be filtered by name"""
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images_detailed({'name': 'new name! #123'})
+        self.assertEquals(len(images), 1)
+
+        for image in images:
+            self.assertEquals('new name! #123', image['name'])
+
+    def test_get_image_details_by_status(self):
+        """Tests that a detailed call can be filtered by status"""
+        extra_fixture = {'id': 3,
+                         'status': 'saving',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images_detailed({'status': 'saving'})
+        self.assertEquals(len(images), 1)
+
+        for image in images:
+            self.assertEquals('saving', image['status'])
+
+    def test_get_image_details_by_container_format(self):
+        """Tests that a detailed call can be filtered by container_format"""
+        extra_fixture = {'id': 3,
+                         'status': 'saving',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images_detailed({'container_format': 'ovf'})
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertEquals('ovf', image['container_format'])
+
+    def test_get_image_details_by_disk_format(self):
+        """Tests that a detailed call can be filtered by disk_format"""
+        extra_fixture = {'id': 3,
+                         'status': 'saving',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images_detailed({'disk_format': 'vhd'})
+        self.assertEquals(len(images), 2)
+
+        for image in images:
+            self.assertEquals('vhd', image['disk_format'])
+
+    def test_get_image_details_with_maximum_size(self):
+        """Tests that a detailed call can be filtered by size_max"""
+        extra_fixture = {'id': 3,
+                         'status': 'saving',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 21,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images_detailed({'size_max': 20})
+        self.assertEquals(len(images), 1)
+
+        for image in images:
+            self.assertTrue(image['size'] <= 20)
+
+    def test_get_image_details_with_minimum_size(self):
+        """Tests that a detailed call can be filtered by size_min"""
+        extra_fixture = {'id': 3,
+                         'status': 'saving',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images_detailed({'size_min': 20})
+        self.assertEquals(len(images), 1)
+
+        for image in images:
+            self.assertTrue(image['size'] >= 20)
+
+    def test_get_image_details_by_property(self):
+        """Tests that a detailed call can be filtered by a property"""
+        extra_fixture = {'id': 3,
+                         'status': 'saving',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None,
+                         'properties': {'p a': 'v a'}}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images_detailed({'property-p a': 'v a'})
+        self.assertEquals(len(images), 1)
+
+        for image in images:
+            self.assertEquals('v a', image['properties']['p a'])
+
     def test_get_image(self):
         """Tests that the detailed info about an image returned"""
         fixture = {'id': 1,