From ca3df19da46e330cd9ce1de77224452177f06071 Mon Sep 17 00:00:00 2001
From: lin-hua-cheng <os.lcheng@gmail.com>
Date: Wed, 19 Nov 2014 17:32:56 -0800
Subject: [PATCH] Always return the service name in the catalog

Fix implemention of service catalog creation in v2, to handle the case
when the service name is not available. Using service.get('name', '')
does not work because that doesn't invoke the __getitem__() from
sql.DictBase parent class of Service. Instead the service name have to
be directly read from the 'extra' attribute of the service object using
service.extra.get('name', '').
The name attribute of the service will now always be included in the
service catalog. The value will be defaulted to empty string if no
service name is available.

Change-Id: I305abd8446fee57af18aebf7873e3c67a1b83c92
Closes-Bug: #1393518
---
 keystone/catalog/backends/sql.py  |  6 ++----
 keystone/tests/test_catalog.py    | 30 ++++++++++++++++++++++++++++
 keystone/tests/test_v3_catalog.py | 33 +++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/keystone/catalog/backends/sql.py b/keystone/catalog/backends/sql.py
index c7860eed24..3cbcc12a31 100644
--- a/keystone/catalog/backends/sql.py
+++ b/keystone/catalog/backends/sql.py
@@ -293,7 +293,7 @@ class Catalog(catalog.Driver):
             service_type = endpoint.service['type']
             default_service = {
                 'id': endpoint['id'],
-                'name': endpoint.service['name'],
+                'name': endpoint.service.extra.get('name', ''),
                 'publicURL': ''
             }
             catalog.setdefault(region, {})
@@ -329,9 +329,7 @@ class Catalog(catalog.Driver):
         def make_v3_service(svc):
             eps = list(make_v3_endpoints(svc.endpoints))
             service = {'endpoints': eps, 'id': svc.id, 'type': svc.type}
-            name = svc.extra.get('name')
-            if name:
-                service['name'] = name
+            service['name'] = svc.extra.get('name', '')
             return service
 
         return [make_v3_service(svc) for svc in services]
diff --git a/keystone/tests/test_catalog.py b/keystone/tests/test_catalog.py
index 32592a14d5..818c253e57 100644
--- a/keystone/tests/test_catalog.py
+++ b/keystone/tests/test_catalog.py
@@ -179,3 +179,33 @@ class TestV2CatalogAPISQL(tests.TestCase):
         self.assertEqual(1, len(catalog))
         # all three endpoints appear in the backend
         self.assertEqual(3, len(self.catalog_api.list_endpoints()))
+
+    def test_get_catalog_always_returns_service_name(self):
+        user_id = uuid.uuid4().hex
+        tenant_id = uuid.uuid4().hex
+
+        # create a service, with a name
+        named_svc = {
+            'id': uuid.uuid4().hex,
+            'type': uuid.uuid4().hex,
+            'name': uuid.uuid4().hex,
+        }
+        self.catalog_api.create_service(named_svc['id'], named_svc)
+        endpoint = self.new_endpoint_ref(service_id=named_svc['id'])
+        self.catalog_api.create_endpoint(endpoint['id'], endpoint)
+
+        # create a service, with no name
+        unnamed_svc = {
+            'id': uuid.uuid4().hex,
+            'type': uuid.uuid4().hex
+        }
+        self.catalog_api.create_service(unnamed_svc['id'], unnamed_svc)
+        endpoint = self.new_endpoint_ref(service_id=unnamed_svc['id'])
+        self.catalog_api.create_endpoint(endpoint['id'], endpoint)
+
+        region = None
+        catalog = self.catalog_api.get_catalog(user_id, tenant_id)
+
+        self.assertEqual(named_svc['name'],
+                         catalog[region][named_svc['type']]['name'])
+        self.assertEqual('', catalog[region][unnamed_svc['type']]['name'])
diff --git a/keystone/tests/test_v3_catalog.py b/keystone/tests/test_v3_catalog.py
index 9b6b591e9d..b79e4aa468 100644
--- a/keystone/tests/test_v3_catalog.py
+++ b/keystone/tests/test_v3_catalog.py
@@ -675,6 +675,39 @@ class TestCatalogAPISQL(tests.TestCase):
         # all three appear in the backend
         self.assertEqual(3, len(self.catalog_api.list_endpoints()))
 
+    def test_get_catalog_always_returns_service_name(self):
+        user_id = uuid.uuid4().hex
+        tenant_id = uuid.uuid4().hex
+
+        # create a service, with a name
+        named_svc = {
+            'id': uuid.uuid4().hex,
+            'type': uuid.uuid4().hex,
+            'name': uuid.uuid4().hex,
+        }
+        self.catalog_api.create_service(named_svc['id'], named_svc)
+        endpoint = self.new_endpoint_ref(service_id=named_svc['id'])
+        self.catalog_api.create_endpoint(endpoint['id'], endpoint)
+
+        # create a service, with no name
+        unnamed_svc = {
+            'id': uuid.uuid4().hex,
+            'type': uuid.uuid4().hex
+        }
+        self.catalog_api.create_service(unnamed_svc['id'], unnamed_svc)
+        endpoint = self.new_endpoint_ref(service_id=unnamed_svc['id'])
+        self.catalog_api.create_endpoint(endpoint['id'], endpoint)
+
+        catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id)
+
+        named_endpoint = [ep for ep in catalog
+                          if ep['type'] == named_svc['type']][0]
+        self.assertEqual(named_svc['name'], named_endpoint['name'])
+
+        unnamed_endpoint = [ep for ep in catalog
+                            if ep['type'] == unnamed_svc['type']][0]
+        self.assertEqual('', unnamed_endpoint['name'])
+
 
 # TODO(dstanek): this needs refactoring with the test above, but we are in a
 # crunch so that will happen in a future patch.