From fb4576eb3fab1ed81ee3c0e3d6bff3b0bd13e78c Mon Sep 17 00:00:00 2001
From: Brian Waldon <brian.waldon@rackspace.com>
Date: Sat, 14 May 2011 21:05:04 -0400
Subject: [PATCH 1/8] implemented api filtering on name, status, disk_format,
 and container_format

---
 glance/api/v1/images.py           |  22 ++-
 glance/registry/__init__.py       |   8 +-
 glance/registry/client.py         |  21 ++-
 glance/registry/db/api.py         |  19 +++
 glance/registry/server.py         |  26 +++-
 tests/functional/test_curl_api.py | 146 ++++++++++++++++++++
 tests/stubs.py                    |   8 ++
 tests/unit/test_api.py            | 217 ++++++++++++++++++++++++++++++
 tests/unit/test_clients.py        |  99 +++++++++++++-
 9 files changed, 550 insertions(+), 16 deletions(-)

diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py
index 581261909f..f15a78f3a0 100644
--- a/glance/api/v1/images.py
+++ b/glance/api/v1/images.py
@@ -42,6 +42,8 @@ from glance import utils
 
 logger = logging.getLogger('glance.api.v1.images')
 
+SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format']
+
 
 class Controller(wsgi.Controller):
 
@@ -89,7 +91,8 @@ class Controller(wsgi.Controller):
                  '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 +117,24 @@ class Controller(wsgi.Controller):
                  '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 SUPPORTED_FILTERS:
+            if param in req.str_params:
+                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
diff --git a/glance/registry/__init__.py b/glance/registry/__init__.py
index a4e4a80b69..44b02528ab 100644
--- a/glance/registry/__init__.py
+++ b/glance/registry/__init__.py
@@ -32,14 +32,14 @@ def get_registry_client(options):
     return client.RegistryClient(host, port)
 
 
-def get_images_list(options):
+def get_images_list(options, filters):
     c = get_registry_client(options)
-    return c.get_images()
+    return c.get_images(filters)
 
 
-def get_images_detail(options):
+def get_images_detail(options, filters):
     c = get_registry_client(options)
-    return c.get_images_detailed()
+    return c.get_images_detailed(filters)
 
 
 def get_image_metadata(options, image_id):
diff --git a/glance/registry/client.py b/glance/registry/client.py
index 336c825f05..9a48a28739 100644
--- a/glance/registry/client.py
+++ b/glance/registry/client.py
@@ -23,9 +23,10 @@ the Glance Registry API
 import httplib
 import json
 import logging
-import urlparse
 import socket
 import sys
+import urlparse
+import urllib
 
 from glance.common import exception
 from glance.client import BaseClient
@@ -49,19 +50,29 @@ class RegistryClient(BaseClient):
         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
 
diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py
index 0e8956b613..ece3fbcc4a 100644
--- a/glance/registry/db/api.py
+++ b/glance/registry/db/api.py
@@ -150,6 +150,25 @@ def image_get_all_public(context):
                    all()
 
 
+def image_get_filtered(context, filters):
+    """Get all public images that match the provided filters.
+
+    :param filters: dict of filter keys and values
+
+    """
+    session = get_session()
+    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')
+
+    for (k, v) in filters.items():
+        query = query.filter(getattr(models.Image, k) == v)
+
+    return query.all()
+
+
 def _drop_protected_attrs(model_class, values):
     """Removed protected attributes from values dictionary using the models
     __protected_attributes__ field.
diff --git a/glance/registry/server.py b/glance/registry/server.py
index 52d17e28fa..3ba95b6609 100644
--- a/glance/registry/server.py
+++ b/glance/registry/server.py
@@ -36,6 +36,8 @@ DISPLAY_FIELDS_IN_INDEX = ['id', 'name', 'size',
                            'disk_format', 'container_format',
                            'checksum']
 
+SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format']
+
 
 class Controller(wsgi.Controller):
     """Controller for the reference implementation registry server"""
@@ -45,7 +47,7 @@ class Controller(wsgi.Controller):
         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 +66,8 @@ class Controller(wsgi.Controller):
             }
 
         """
-        images = db_api.image_get_all_public(None)
+        images = db_api.image_get_filtered(None, self._get_filters(req))
+
         results = []
         for image in images:
             result = {}
@@ -74,7 +77,7 @@ class Controller(wsgi.Controller):
         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 +88,25 @@ class Controller(wsgi.Controller):
         all image model fields.
 
         """
-        images = db_api.image_get_all_public(None)
+        images = db_api.image_get_filtered(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 = {}
+        for param in SUPPORTED_FILTERS:
+            if param in req.str_params:
+                filters[param] = req.str_params.get(param)
+
+        return filters
+
     def show(self, req, id):
         """Return data about the given image id."""
         try:
diff --git a/tests/functional/test_curl_api.py b/tests/functional/test_curl_api.py
index 099ead4c94..1d3deb33f9 100644
--- a/tests/functional/test_curl_api.py
+++ b/tests/functional/test_curl_api.py
@@ -787,3 +787,149 @@ class TestCurlApi(functional.FunctionalTest):
                             "Could not find '%s' in '%s'" % (expected, out))
 
         self.stop_servers()
+
+    def test_filtered_images(self):
+        """
+
+        """
+        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-Is-Public: True' "
+               "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-Is-Public: True' "
+               "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-Is-Public: True' "
+               "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")
diff --git a/tests/stubs.py b/tests/stubs.py
index 6bb8670b9a..f914aca554 100644
--- a/tests/stubs.py
+++ b/tests/stubs.py
@@ -390,6 +390,12 @@ def stub_out_registry_db_image_api(stubs):
             return [f for f in self.images
                     if f['is_public'] == public]
 
+        def image_get_filtered(self, _context, filters):
+            images = [f for f in self.images if f['is_public'] == True]
+            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',
               fake_datastore.image_create)
@@ -401,3 +407,5 @@ def stub_out_registry_db_image_api(stubs):
               fake_datastore.image_get)
     stubs.Set(glance.registry.db.api, 'image_get_all_public',
               fake_datastore.image_get_all_public)
+    stubs.Set(glance.registry.db.api, 'image_get_filtered',
+              fake_datastore.image_get_filtered)
diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py
index 3f605b609b..0e23f9393f 100644
--- a/tests/unit/test_api.py
+++ b/tests/unit/test_api.py
@@ -26,6 +26,7 @@ import webob
 
 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 @@ class TestRegistryAPI(unittest.TestCase):
         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,178 @@ class TestRegistryAPI(unittest.TestCase):
         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
+
+        """
+        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/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
+
+        """
+        fixture = {'id': 2,
+                   'name': 'fake image #2',
+                   'size': 19,
+                   'checksum': None}
+
+        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
+
+        """
+        fixture = {'id': 2,
+                   'name': 'fake image #2',
+                   'size': 19,
+                   'checksum': None}
+
+        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
+
+        """
+        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': '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_create_image(self):
         """Tests that the /images POST registry API creates the image"""
         fixture = {'name': 'fake public image',
diff --git a/tests/unit/test_clients.py b/tests/unit/test_clients.py
index b01d77a615..236b243880 100644
--- a/tests/unit/test_clients.py
+++ b/tests/unit/test_clients.py
@@ -24,8 +24,9 @@ import unittest
 import webob
 
 from glance import client
-from glance.registry import client as rclient
 from glance.common import exception
+import glance.registry.db.api
+from glance.registry import client as rclient
 from tests import stubs
 
 
@@ -69,6 +70,26 @@ class TestRegistryClient(unittest.TestCase):
         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,82 @@ class TestRegistryClient(unittest.TestCase):
         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(self):
         """Tests that the detailed info about an image returned"""
         fixture = {'id': 1,

From 259d853bf2f18f72988dad4a58c0d0ef4864977f Mon Sep 17 00:00:00 2001
From: Brian Waldon <brian.waldon@rackspace.com>
Date: Sun, 15 May 2011 20:34:35 -0400
Subject: [PATCH 2/8] adding size_min and size_max api query filters

---
 glance/api/v1/images.py           |   3 +-
 glance/registry/db/api.py         |   8 ++
 glance/registry/server.py         |   3 +-
 tests/functional/test_curl_api.py |  33 ++++++-
 tests/stubs.py                    |  10 +++
 tests/unit/test_api.py            | 141 ++++++++++++++++++++++++++++++
 tests/unit/test_clients.py        |  38 ++++++++
 7 files changed, 233 insertions(+), 3 deletions(-)

diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py
index f15a78f3a0..4d63ce06fc 100644
--- a/glance/api/v1/images.py
+++ b/glance/api/v1/images.py
@@ -42,7 +42,8 @@ from glance import utils
 
 logger = logging.getLogger('glance.api.v1.images')
 
-SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format']
+SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format',
+                     'size_min', 'size_max']
 
 
 class Controller(wsgi.Controller):
diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py
index ece3fbcc4a..e4f99905fe 100644
--- a/glance/registry/db/api.py
+++ b/glance/registry/db/api.py
@@ -163,6 +163,14 @@ def image_get_filtered(context, filters):
                    filter_by(is_public=True).\
                    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.items():
         query = query.filter(getattr(models.Image, k) == v)
 
diff --git a/glance/registry/server.py b/glance/registry/server.py
index 3ba95b6609..1dd9e63cb8 100644
--- a/glance/registry/server.py
+++ b/glance/registry/server.py
@@ -36,7 +36,8 @@ DISPLAY_FIELDS_IN_INDEX = ['id', 'name', 'size',
                            'disk_format', 'container_format',
                            'checksum']
 
-SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format']
+SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format',
+                     'size_min', 'size_max']
 
 
 class Controller(wsgi.Controller):
diff --git a/tests/functional/test_curl_api.py b/tests/functional/test_curl_api.py
index 1d3deb33f9..26b122e43d 100644
--- a/tests/functional/test_curl_api.py
+++ b/tests/functional/test_curl_api.py
@@ -790,7 +790,7 @@ class TestCurlApi(functional.FunctionalTest):
 
     def test_filtered_images(self):
         """
-
+        Set up three test images and ensure each query param filter works
         """
         self.cleanup()
         self.start_servers()
@@ -814,6 +814,7 @@ class TestCurlApi(functional.FunctionalTest):
                "-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' "
                "http://0.0.0.0:%d/v1/images") % api_port
 
@@ -831,6 +832,7 @@ class TestCurlApi(functional.FunctionalTest):
                "-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' "
                "http://0.0.0.0:%d/v1/images") % api_port
 
@@ -847,6 +849,7 @@ class TestCurlApi(functional.FunctionalTest):
                "-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' "
                "http://0.0.0.0:%d/v1/images") % api_port
 
@@ -933,3 +936,31 @@ class TestCurlApi(functional.FunctionalTest):
         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)
diff --git a/tests/stubs.py b/tests/stubs.py
index f914aca554..c837e5c8ea 100644
--- a/tests/stubs.py
+++ b/tests/stubs.py
@@ -392,8 +392,18 @@ def stub_out_registry_db_image_api(stubs):
 
         def image_get_filtered(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]
+
             for k, v in filters.items():
                 images = [f for f in images if f[k] == v]
+
             return images
 
     fake_datastore = FakeDatastore()
diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py
index 0e23f9393f..7b0c56becf 100644
--- a/tests/unit/test_api.py
+++ b/tests/unit/test_api.py
@@ -329,6 +329,147 @@ class TestRegistryAPI(unittest.TestCase):
         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
+
+        """
+        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': '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
+
+        """
+        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': '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
+
+        """
+        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': '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_create_image(self):
         """Tests that the /images POST registry API creates the image"""
         fixture = {'name': 'fake public image',
diff --git a/tests/unit/test_clients.py b/tests/unit/test_clients.py
index 236b243880..5653f09aca 100644
--- a/tests/unit/test_clients.py
+++ b/tests/unit/test_clients.py
@@ -184,6 +184,44 @@ class TestRegistryClient(unittest.TestCase):
         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(self):
         """Tests that the detailed info about an image returned"""
         fixture = {'id': 1,

From fb501c459927b91b2f71a8e34b66adb47bc4aa21 Mon Sep 17 00:00:00 2001
From: Brian Waldon <brian.waldon@rackspace.com>
Date: Sun, 15 May 2011 22:16:49 -0400
Subject: [PATCH 3/8] adding custom property api filtering

---
 glance/api/v1/images.py           |  4 +-
 glance/registry/db/api.py         | 23 +++++++++-
 glance/registry/server.py         |  4 +-
 tests/functional/test_curl_api.py | 17 +++++++
 tests/stubs.py                    | 14 +++++-
 tests/unit/test_api.py            | 75 ++++++++++++++++---------------
 tests/unit/test_clients.py        | 20 +++++++++
 7 files changed, 115 insertions(+), 42 deletions(-)

diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py
index 4d63ce06fc..ff582d9ffe 100644
--- a/glance/api/v1/images.py
+++ b/glance/api/v1/images.py
@@ -130,8 +130,8 @@ class Controller(wsgi.Controller):
 
         """
         filters = {}
-        for param in SUPPORTED_FILTERS:
-            if param in req.str_params:
+        for param in req.str_params:
+            if param in SUPPORTED_FILTERS or param.startswith('property-'):
                 filters[param] = req.str_params.get(param)
 
         return filters
diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py
index e4f99905fe..14aa6fbe1c 100644
--- a/glance/registry/db/api.py
+++ b/glance/registry/db/api.py
@@ -172,9 +172,28 @@ def image_get_filtered(context, filters):
         del filters['size_max']
 
     for (k, v) in filters.items():
-        query = query.filter(getattr(models.Image, k) == v)
+        if not k.startswith('property-'):
+            query = query.filter(getattr(models.Image, k) == v)
 
-    return query.all()
+    images = query.all()
+
+    #TODO(bcwaldon): use an actual sqlalchemy query to accomplish this
+    def prop_filter(key, value):
+        def func(image):
+            for prop in image.properties:
+                if prop.deleted == False and \
+                   prop.name == key and \
+                   prop.value == value:
+                    return True
+            return False
+        return func
+
+    for (k, v) in filters.items():
+        if k.startswith('property-'):
+            _k = k[9:]
+            images = filter(prop_filter(_k, v), images)
+
+    return images
 
 
 def _drop_protected_attrs(model_class, values):
diff --git a/glance/registry/server.py b/glance/registry/server.py
index 1dd9e63cb8..6312343aa6 100644
--- a/glance/registry/server.py
+++ b/glance/registry/server.py
@@ -102,8 +102,8 @@ class Controller(wsgi.Controller):
 
         """
         filters = {}
-        for param in SUPPORTED_FILTERS:
-            if param in req.str_params:
+        for param in req.str_params:
+            if param in SUPPORTED_FILTERS or param.startswith('property-'):
                 filters[param] = req.str_params.get(param)
 
         return filters
diff --git a/tests/functional/test_curl_api.py b/tests/functional/test_curl_api.py
index 26b122e43d..0b263e19e3 100644
--- a/tests/functional/test_curl_api.py
+++ b/tests/functional/test_curl_api.py
@@ -816,6 +816,7 @@ class TestCurlApi(functional.FunctionalTest):
                "-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)
@@ -834,6 +835,7 @@ class TestCurlApi(functional.FunctionalTest):
                "-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)
@@ -851,6 +853,7 @@ class TestCurlApi(functional.FunctionalTest):
                "-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)
@@ -964,3 +967,17 @@ class TestCurlApi(functional.FunctionalTest):
         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")
diff --git a/tests/stubs.py b/tests/stubs.py
index c837e5c8ea..487b716eb3 100644
--- a/tests/stubs.py
+++ b/tests/stubs.py
@@ -401,8 +401,20 @@ def stub_out_registry_db_image_api(stubs):
                 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.items():
-                images = [f for f in images if f[k] == v]
+                if k.startswith('property-'):
+                    _k = k[9:]
+                    images = filter(_prop_filter(_k, v), images)
+                else:
+                    images = [f for f in images if f[k] == v]
 
             return images
 
diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py
index 7b0c56becf..1c046247f0 100644
--- a/tests/unit/test_api.py
+++ b/tests/unit/test_api.py
@@ -162,11 +162,6 @@ class TestRegistryAPI(unittest.TestCase):
         public images that have a specific name
 
         """
-        fixture = {'id': 2,
-                   'name': 'fake image #2',
-                   'size': 19,
-                   'checksum': None}
-
         extra_fixture = {'id': 3,
                          'status': 'active',
                          'is_public': True,
@@ -205,11 +200,6 @@ class TestRegistryAPI(unittest.TestCase):
         public images that have a specific status
 
         """
-        fixture = {'id': 2,
-                   'name': 'fake image #2',
-                   'size': 19,
-                   'checksum': None}
-
         extra_fixture = {'id': 3,
                          'status': 'saving',
                          'is_public': True,
@@ -248,11 +238,6 @@ class TestRegistryAPI(unittest.TestCase):
         public images that have a specific container_format
 
         """
-        fixture = {'id': 2,
-                   'name': 'fake image #2',
-                   'size': 19,
-                   'checksum': None}
-
         extra_fixture = {'id': 3,
                          'status': 'active',
                          'is_public': True,
@@ -291,11 +276,6 @@ class TestRegistryAPI(unittest.TestCase):
         public images that have a specific disk_format
 
         """
-        fixture = {'id': 2,
-                   'name': 'fake image #2',
-                   'size': 19,
-                   'checksum': None}
-
         extra_fixture = {'id': 3,
                          'status': 'active',
                          'is_public': True,
@@ -334,11 +314,6 @@ class TestRegistryAPI(unittest.TestCase):
         public images that have a size greater than or equal to size_min
 
         """
-        fixture = {'id': 2,
-                   'name': 'fake image #2',
-                   'size': 19,
-                   'checksum': None}
-
         extra_fixture = {'id': 3,
                          'status': 'active',
                          'is_public': True,
@@ -377,11 +352,6 @@ class TestRegistryAPI(unittest.TestCase):
         public images that have a size less than or equal to size_max
 
         """
-        fixture = {'id': 2,
-                   'name': 'fake image #2',
-                   'size': 19,
-                   'checksum': None}
-
         extra_fixture = {'id': 3,
                          'status': 'active',
                          'is_public': True,
@@ -421,11 +391,6 @@ class TestRegistryAPI(unittest.TestCase):
         and greater than or equal to size_min
 
         """
-        fixture = {'id': 2,
-                   'name': 'fake image #2',
-                   'size': 19,
-                   'checksum': None}
-
         extra_fixture = {'id': 3,
                          'status': 'active',
                          'is_public': True,
@@ -470,6 +435,46 @@ class TestRegistryAPI(unittest.TestCase):
         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',
diff --git a/tests/unit/test_clients.py b/tests/unit/test_clients.py
index 5653f09aca..983b0517d6 100644
--- a/tests/unit/test_clients.py
+++ b/tests/unit/test_clients.py
@@ -222,6 +222,26 @@ class TestRegistryClient(unittest.TestCase):
         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,

From 60bfbbdab096c8f74ec7fd691e08731027beccb2 Mon Sep 17 00:00:00 2001
From: Brian Waldon <brian.waldon@rackspace.com>
Date: Sun, 15 May 2011 22:38:44 -0400
Subject: [PATCH 4/8] adding test case for multiple parameters from command
 line

---
 tests/functional/test_curl_api.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/functional/test_curl_api.py b/tests/functional/test_curl_api.py
index 0b263e19e3..b08cd052a1 100644
--- a/tests/functional/test_curl_api.py
+++ b/tests/functional/test_curl_api.py
@@ -981,3 +981,19 @@ class TestCurlApi(functional.FunctionalTest):
         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!")

From e40fb50f28c90dd2e08cbd02d082d41bd3ba9beb Mon Sep 17 00:00:00 2001
From: Brian Waldon <brian.waldon@rackspace.com>
Date: Mon, 16 May 2011 09:02:09 -0400
Subject: [PATCH 5/8] consolidating image_get_all_public and image_get_filtered
 in registry db api

---
 glance/registry/db/api.py | 18 +++++-------------
 glance/registry/server.py |  4 ++--
 tests/stubs.py            |  8 +-------
 3 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py
index 14aa6fbe1c..1da91a3e1e 100644
--- a/glance/registry/db/api.py
+++ b/glance/registry/db/api.py
@@ -139,23 +139,15 @@ def image_get(context, image_id, session=None):
         raise exception.NotFound("No image found with ID %s" % image_id)
 
 
-def image_get_all_public(context):
-    """Get all public images."""
-    session = get_session()
-    return 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()
-
-
-def image_get_filtered(context, filters):
-    """Get all public images that match the provided filters.
+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 filters == None:
+        filters = {}
+
     session = get_session()
     query = session.query(models.Image).\
                    options(joinedload(models.Image.properties)).\
diff --git a/glance/registry/server.py b/glance/registry/server.py
index 6312343aa6..9cb9c7eeb9 100644
--- a/glance/registry/server.py
+++ b/glance/registry/server.py
@@ -67,7 +67,7 @@ class Controller(wsgi.Controller):
             }
 
         """
-        images = db_api.image_get_filtered(None, self._get_filters(req))
+        images = db_api.image_get_all_public(None, self._get_filters(req))
 
         results = []
         for image in images:
@@ -89,7 +89,7 @@ class Controller(wsgi.Controller):
         all image model fields.
 
         """
-        images = db_api.image_get_filtered(None, self._get_filters(req))
+        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)
diff --git a/tests/stubs.py b/tests/stubs.py
index 487b716eb3..422a490271 100644
--- a/tests/stubs.py
+++ b/tests/stubs.py
@@ -386,11 +386,7 @@ def stub_out_registry_db_image_api(stubs):
             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_filtered(self, _context, filters):
+        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:
@@ -429,5 +425,3 @@ def stub_out_registry_db_image_api(stubs):
               fake_datastore.image_get)
     stubs.Set(glance.registry.db.api, 'image_get_all_public',
               fake_datastore.image_get_all_public)
-    stubs.Set(glance.registry.db.api, 'image_get_filtered',
-              fake_datastore.image_get_filtered)

From 45eb5582b3d4adad2f01974e9ed3d372bb34f300 Mon Sep 17 00:00:00 2001
From: Brian Waldon <brian.waldon@rackspace.com>
Date: Mon, 16 May 2011 09:20:53 -0400
Subject: [PATCH 6/8] making registry db api filters more structured; adding in
 a bit of sqlalchemy code to filter image properties more efficiently

---
 glance/registry/db/api.py | 28 +++++++---------------------
 glance/registry/server.py | 10 +++++++++-
 tests/stubs.py            |  9 ++++-----
 3 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py
index 1da91a3e1e..6abc277dad 100644
--- a/glance/registry/db/api.py
+++ b/glance/registry/db/api.py
@@ -142,7 +142,9 @@ def image_get(context, image_id, session=None):
 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
+    :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:
@@ -163,29 +165,13 @@ def image_get_all_public(context, filters=None):
         query = query.filter(models.Image.size <= filters['size_max'])
         del filters['size_max']
 
-    for (k, v) in filters.items():
-        if not k.startswith('property-'):
-            query = query.filter(getattr(models.Image, k) == v)
-
-    images = query.all()
-
-    #TODO(bcwaldon): use an actual sqlalchemy query to accomplish this
-    def prop_filter(key, value):
-        def func(image):
-            for prop in image.properties:
-                if prop.deleted == False and \
-                   prop.name == key and \
-                   prop.value == value:
-                    return True
-            return False
-        return func
+    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():
-        if k.startswith('property-'):
-            _k = k[9:]
-            images = filter(prop_filter(_k, v), images)
+        query = query.filter(getattr(models.Image, k) == v)
 
-    return images
+    return query.all()
 
 
 def _drop_protected_attrs(model_class, values):
diff --git a/glance/registry/server.py b/glance/registry/server.py
index 9cb9c7eeb9..44030d7a96 100644
--- a/glance/registry/server.py
+++ b/glance/registry/server.py
@@ -102,9 +102,17 @@ class Controller(wsgi.Controller):
 
         """
         filters = {}
+        properties = {}
+
         for param in req.str_params:
-            if param in SUPPORTED_FILTERS or param.startswith('property-'):
+            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
 
diff --git a/tests/stubs.py b/tests/stubs.py
index 422a490271..c7816ac66d 100644
--- a/tests/stubs.py
+++ b/tests/stubs.py
@@ -405,12 +405,11 @@ def stub_out_registry_db_image_api(stubs):
                     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():
-                if k.startswith('property-'):
-                    _k = k[9:]
-                    images = filter(_prop_filter(_k, v), images)
-                else:
-                    images = [f for f in images if f[k] == v]
+                images = [f for f in images if f[k] == v]
 
             return images
 

From d07be5ab9487e0e3acd645b1de8e2dffaa440602 Mon Sep 17 00:00:00 2001
From: Brian Waldon <brian.waldon@rackspace.com>
Date: Mon, 16 May 2011 10:28:41 -0400
Subject: [PATCH 7/8] removing some unnecessary imports

---
 glance/registry/client.py | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/glance/registry/client.py b/glance/registry/client.py
index 9a48a28739..7de4cdf375 100644
--- a/glance/registry/client.py
+++ b/glance/registry/client.py
@@ -20,15 +20,9 @@ Simple client class to speak with any RESTful service that implements
 the Glance Registry API
 """
 
-import httplib
 import json
-import logging
-import socket
-import sys
-import urlparse
 import urllib
 
-from glance.common import exception
 from glance.client import BaseClient
 
 
@@ -77,11 +71,7 @@ class RegistryClient(BaseClient):
         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

From 32fa5ad441979d7c67cfbf378bc5410f75fdb07d Mon Sep 17 00:00:00 2001
From: Brian Waldon <brian.waldon@rackspace.com>
Date: Tue, 17 May 2011 09:27:09 -0400
Subject: [PATCH 8/8] docstring fix

---
 glance/api/v1/images.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py
index ff582d9ffe..62a1ed90f4 100644
--- a/glance/api/v1/images.py
+++ b/glance/api/v1/images.py
@@ -123,11 +123,11 @@ class Controller(wsgi.Controller):
         return dict(images=images)
 
     def _get_filters(self, req):
-        """Return a dictionary of query param filters from the request
+        """
+        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: