Refactor 404's into managers & drivers (bug 968519)

The goal is to move the responsibility of reference checks away from
controllers and into the underlying managers & drivers, which can
handle the task with equal or greater efficiency.

- Tenant references from create_user/update_user are NOT tested
  due to inconsistencies between backends
- Additional test coverage improvements

Also fixes bug 999209, bug 999608, bug 1006029, bug 1006055, bug 1006287,
bug 1006334, and bug 1006344.

Change-Id: I7de592e7dd4518038436b9a9fdaab559b00a0537

3 files changed, 75 insertions, 36 deletions

diff --git a/keystone/catalog/backends/kvs.py b/keystone/catalog/backends/kvs.py
index f92a949a..e63a75ff 100644
--- a/keystone/catalog/backends/kvs.py
+++ b/keystone/catalog/backends/kvs.py
@@ -16,7 +16,6 @@
 
 
 from keystone import catalog
-from keystone import exception
 from keystone.common import kvs
 
 
@@ -26,10 +25,7 @@ class Catalog(kvs.Base, catalog.Driver):
         return self.db.get('catalog-%s-%s' % (tenant_id, user_id))
 
     def get_service(self, service_id):
-        res = self.db.get('service-%s' % service_id)
-        if not res:
-            raise exception.ServiceNotFound(service_id=service_id)
-        return res
+        return self.db.get('service-%s' % service_id)
 
     def list_services(self):
         return self.db.get('service_list', [])
@@ -46,14 +42,10 @@ class Catalog(kvs.Base, catalog.Driver):
         return service
 
     def delete_service(self, service_id):
-        if not self.db.get('service-%s' % service_id):
-            raise exception.ServiceNotFound(service_id=service_id)
-
         self.db.delete('service-%s' % service_id)
         service_list = set(self.db.get('service_list', []))
         service_list.remove(service_id)
         self.db.set('service_list', list(service_list))
-        return None
 
     # Private interface
     def _create_catalog(self, user_id, tenant_id, data):
diff --git a/keystone/catalog/backends/sql.py b/keystone/catalog/backends/sql.py
index 33dd59f1..785baeaf 100644
--- a/keystone/catalog/backends/sql.py
+++ b/keystone/catalog/backends/sql.py
@@ -93,11 +93,9 @@ class Catalog(sql.Base, catalog.Driver):
 
     def delete_service(self, service_id):
         session = self.get_session()
-        service_ref = session.query(Service).filter_by(id=service_id).first()
-        if not service_ref:
-            raise exception.ServiceNotFound(service_id=service_id)
         with session.begin():
-            session.delete(service_ref)
+            if not session.query(Service).filter_by(id=service_id).delete():
+                raise exception.ServiceNotFound(service_id=service_id)
             session.flush()
 
     def create_service(self, service_id, service_ref):
@@ -111,6 +109,7 @@ class Catalog(sql.Base, catalog.Driver):
     # Endpoints
     def create_endpoint(self, endpoint_id, endpoint_ref):
         session = self.get_session()
+        self.get_service(endpoint_ref['service_id'])
         new_endpoint = Endpoint.from_dict(endpoint_ref)
         with session.begin():
             session.add(new_endpoint)
@@ -119,12 +118,9 @@ class Catalog(sql.Base, catalog.Driver):
 
     def delete_endpoint(self, endpoint_id):
         session = self.get_session()
-        endpoint_ref = session.query(Endpoint)
-        endpoint_ref = endpoint_ref.filter_by(id=endpoint_id).first()
-        if not endpoint_ref:
-            raise exception.EndpointNotFound(endpoint_id=endpoint_id)
         with session.begin():
-            session.delete(endpoint_ref)
+            if not session.query(Endpoint).filter_by(id=endpoint_id).delete():
+                raise exception.EndpointNotFound(endpoint_id=endpoint_id)
             session.flush()
 
     def get_endpoint(self, endpoint_id):
diff --git a/keystone/catalog/core.py b/keystone/catalog/core.py
index bc3e72be..b2b6c24e 100644
--- a/keystone/catalog/core.py
+++ b/keystone/catalog/core.py
@@ -42,13 +42,50 @@ class Manager(manager.Manager):
     def __init__(self):
         super(Manager, self).__init__(CONF.catalog.driver)
 
+    def get_service(self, context, service_id):
+        try:
+            return self.driver.get_service(service_id)
+        except exception.NotFound:
+            raise exception.ServiceNotFound(service_id=service_id)
+
+    def delete_service(self, context, service_id):
+        try:
+            return self.driver.delete_service(service_id)
+        except exception.NotFound:
+            raise exception.ServiceNotFound(service_id=service_id)
+
+    def create_endpoint(self, context, endpoint_id, endpoint_ref):
+        try:
+            return self.driver.create_endpoint(endpoint_id, endpoint_ref)
+        except exception.NotFound:
+            service_id = endpoint_ref.get('service_id')
+            raise exception.ServiceNotFound(service_id=service_id)
+
+    def delete_endpoint(self, context, endpoint_id):
+        try:
+            return self.driver.delete_endpoint(endpoint_id)
+        except exception.NotFound:
+            raise exception.EndpointNotFound(endpoint_id=endpoint_id)
+
+    def get_endpoint(self, context, endpoint_id):
+        try:
+            return self.driver.get_endpoint(endpoint_id)
+        except exception.NotFound:
+            raise exception.EndpointNotFound(endpoint_id=endpoint_id)
+
+    def get_catalog(self, context, user_id, tenant_id, metadata=None):
+        try:
+            return self.driver.get_catalog(user_id, tenant_id, metadata)
+        except exception.NotFound:
+            raise exception.NotFound('Catalog not found for user and tenant')
+
 
 class Driver(object):
     """Interface description for an Catalog driver."""
     def list_services(self):
         """List all service ids in catalog.
 
-        Returns: list of service_ids or an empty list.
+        :returns: list of service_ids or an empty list.
 
         """
         raise exception.NotImplemented()
@@ -56,27 +93,50 @@ class Driver(object):
     def get_service(self, service_id):
         """Get service by id.
 
-        Returns: service_ref dict or None.
+        :returns: service_ref dict
+        :raises: keystone.exception.ServiceNotFound
 
         """
         raise exception.NotImplemented()
 
     def delete_service(self, service_id):
+        """Deletes an existing service.
+
+        :raises: keystone.exception.ServiceNotFound
+
+        """
         raise exception.NotImplemented()
 
     def create_service(self, service_id, service_ref):
+        """Creates a new service.
+
+        :raises: keystone.exception.Conflict
+
+        """
         raise exception.NotImplemented()
 
     def create_endpoint(self, endpoint_id, endpoint_ref):
+        """Creates a new endpoint for a service.
+
+        :raises: keystone.exception.Conflict,
+                 keystone.exception.ServiceNotFound
+
+        """
         raise exception.NotImplemented()
 
     def delete_endpoint(self, endpoint_id):
+        """Deletes an endpoint for a service.
+
+        :raises: keystone.exception.EndpointNotFound
+
+        """
         raise exception.NotImplemented()
 
     def get_endpoint(self, endpoint_id):
         """Get endpoint by id.
 
-        Returns: endpoint_ref dict or None.
+        :returns: endpoint_ref dict
+        :raises: keystone.exception.EndpointNotFound
 
         """
         raise exception.NotImplemented()
@@ -84,7 +144,7 @@ class Driver(object):
     def list_endpoints(self):
         """List all endpoint ids in catalog.
 
-        Returns: list of endpoint_ids or an empty list.
+        :returns: list of endpoint_ids or an empty list.
 
         """
         raise exception.NotImplemented()
@@ -92,10 +152,7 @@ class Driver(object):
     def get_catalog(self, user_id, tenant_id, metadata=None):
         """Retreive and format the current service catalog.
 
-        Returns: A nested dict representing the service catalog or an
-                 empty dict.
-
-        Example:
+        Example::
 
             { 'RegionOne':
                 {'compute': {
@@ -109,6 +166,10 @@ class Driver(object):
                     'name': 'EC2 Service',
                     'publicURL': 'http://host:8773/services/Cloud'}}
 
+        :returns: A nested dict representing the service catalog or an
+                  empty dict.
+        :raises: keystone.exception.NotFound
+
         """
         raise exception.NotImplemented()
 
@@ -133,15 +194,10 @@ class ServiceController(wsgi.Application):
     def get_service(self, context, service_id):
         self.assert_admin(context)
         service_ref = self.catalog_api.get_service(context, service_id)
-        if not service_ref:
-            raise exception.ServiceNotFound(service_id=service_id)
         return {'OS-KSADM:service': service_ref}
 
     def delete_service(self, context, service_id):
         self.assert_admin(context)
-        service_ref = self.catalog_api.get_service(context, service_id)
-        if not service_ref:
-            raise exception.ServiceNotFound(service_id=service_id)
         self.catalog_api.delete_service(context, service_id)
 
     def create_service(self, context, OS_KSADM_service):
@@ -174,11 +230,6 @@ class EndpointController(wsgi.Application):
         endpoint_id = uuid.uuid4().hex
         endpoint_ref = endpoint.copy()
         endpoint_ref['id'] = endpoint_id
-
-        service_id = endpoint_ref['service_id']
-        if not self.catalog_api.get_service(context, service_id):
-            raise exception.ServiceNotFound(service_id=service_id)
-
         new_endpoint_ref = self.catalog_api.create_endpoint(
             context, endpoint_id, endpoint_ref)
         return {'endpoint': new_endpoint_ref}