Improve ShareServer DB model

Currently, some DB API methods related to share_servers
manually retrieve backend_details via
share_server_backend_details_get() method. It's bad practice
because some DB API methods return share_server models with
loaded backend_details and other models return models without
backend_details. It forces developers to manually check that
share_server model has backend_details attribute.

- Automatically load read-only backend_details to ShareServer model
- Remove redundant calls of share_server_backend_details_get() method
- Remove share_server_backend_details_get() method
- Update appropriate unit tests

Change-Id: Ida9a1939819d9b6f4bc7aed5eecd4592627c4e3a

manila/api/v1/share_servers.py

@@ -85,11 +85,12 @@ class ShareServerController(wsgi.Controller):
         context = req.environ['manila.context']
         policy.check_policy(context, RESOURCE_NAME, 'details')
         try:
-            db_api.share_server_get(context, id)
+            share_server = db_api.share_server_get(context, id)
         except exception.ShareServerNotFound as e:
             raise exc.HTTPNotFound(explanation=six.text_type(e))
-        details = db_api.share_server_backend_details_get(context, id)
-        return self._view_builder.build_share_server_details(details)
+
+        return self._view_builder.build_share_server_details(
+            share_server['backend_details'])
 
     def delete(self, req, id):
         """Delete specified share server."""

manila/db/api.py

@@ -652,11 +652,6 @@ def share_server_backend_details_set(context, share_server_id, server_details):
                                                  server_details)
 
 
-def share_server_backend_details_get(context, share_server_id):
-    """Get all backend details records for share server."""
-    return IMPL.share_server_backend_details_get(context, share_server_id)
-
-
 ##################
 
 

manila/db/sqlalchemy/api.py

@@ -1966,7 +1966,8 @@ def share_server_create(context, values):
     session = get_session()
     with session.begin():
         server_ref.save(session=session)
-    return server_ref
+        # NOTE(u_glide): Do so to prevent errors with relationships
+        return share_server_get(context, server_ref['id'], session=session)
 
 
 @require_context
@@ -1997,8 +1998,6 @@ def share_server_get(context, server_id, session=None):
         .first()
     if result is None:
         raise exception.ShareServerNotFound(share_server_id=server_id)
-    result['backend_details'] = share_server_backend_details_get(
-        context, server_id, session=session)
     return result
 
 
@@ -2020,8 +2019,6 @@ def share_server_get_by_host_and_share_net_valid(context, host, share_net_id,
         }
         raise exception.ShareServerNotFoundByFilters(
             filters_description=filters_description)
-    result['backend_details'] = share_server_backend_details_get(
-        context, result['id'], session=session)
     return result
 
 
@@ -2075,17 +2072,6 @@ def share_server_backend_details_delete(context, share_server_id,
         item.soft_delete(session)
 
 
-@require_context
-def share_server_backend_details_get(context, share_server_id,
-                                     session=None):
-    if not session:
-        session = get_session()
-    query = model_query(context, models.ShareServerBackendDetails,
-                        session=session)\
-        .filter_by(share_server_id=share_server_id).all()
-    return dict([(item.key, item.value) for item in query])
-
-
 ###################
 
 

manila/db/sqlalchemy/models.py

@@ -417,6 +417,22 @@ class ShareServer(BASE, ManilaBase):
                               'ShareServer.id == Share.share_server_id,'
                               'Share.deleted == "False")')
 
+    _backend_details = orm.relationship(
+        "ShareServerBackendDetails",
+        lazy='immediate',
+        viewonly=True,
+        primaryjoin='and_('
+                    'ShareServer.id == '
+                    'ShareServerBackendDetails.share_server_id, '
+                    'ShareServerBackendDetails.deleted == "False")')
+
+    @property
+    def backend_details(self):
+        return dict((model['key'], model['value'])
+                    for model in self._backend_details)
+
+    _extra_keys = ['backend_details']
+
 
 class ShareServerBackendDetails(BASE, ManilaBase):
     """Represents a metadata key/value pair for a share server."""

manila/share/manager.py

@@ -796,11 +796,7 @@ class ShareManager(manager.SchedulerDependentManager):
             if shares:
                 raise exception.ShareServerInUse(share_server_id=server_id)
 
-            if 'backend_details' not in share_server:
-                server_details = self.db.share_server_backend_details_get(
-                    context, server_id)
-            else:
-                server_details = share_server['backend_details']
+            server_details = share_server['backend_details']
 
             self.db.share_server_update(context, server_id,
                                         {'status': constants.STATUS_DELETING})

manila/tests/api/v1/test_share_servers.py

@@ -119,10 +119,6 @@ def fake_share_server_get():
     return FakeShareServer(created_at=None)
 
 
-def fake_share_server_backend_details_get():
-    return share_server_backend_details
-
-
 class FakeRequestAdmin(object):
     environ = {"manila.context": CONTEXT}
     GET = {}
@@ -236,9 +232,6 @@ class ShareServerAPITest(test.TestCase):
     def test_details(self):
         self.mock_object(db_api, 'share_server_get',
                          mock.Mock(return_value=fake_share_server_get()))
-        self.mock_object(
-            db_api, 'share_server_backend_details_get',
-            mock.Mock(return_value=fake_share_server_backend_details_get()))
         result = self.controller.details(
             FakeRequestAdmin,
             fake_share_server_get_result['share_server']['id'])
@@ -246,8 +239,6 @@ class ShareServerAPITest(test.TestCase):
             CONTEXT, share_servers.RESOURCE_NAME, 'details')
         db_api.share_server_get.assert_called_once_with(
             CONTEXT, fake_share_server_get_result['share_server']['id'])
-        db_api.share_server_backend_details_get.assert_called_once_with(
-            CONTEXT, fake_share_server_get_result['share_server']['id'])
         self.assertEqual(result,
                          fake_share_server_backend_details_get_result)
 

manila/tests/test_db.py

@@ -175,7 +175,7 @@ class ShareServerTableTestCase(test.TestCase):
 
         self.assertDictMatch(
             details,
-            db.share_server_backend_details_get(self.ctxt, server['id'])
+            db.share_server_get(self.ctxt, server['id'])['backend_details']
         )
 
     def test_share_server_backend_details_set_not_found(self):
@@ -202,6 +202,7 @@ class ShareServerTableTestCase(test.TestCase):
         self.assertEqual(server.host, values['host'])
         self.assertEqual(server.status, values['status'])
         self.assertDictMatch(details, server['backend_details'])
+        self.assertTrue('backend_details' in server.to_dict())
 
     def test_share_server_delete_with_details(self):
         server = self._create_share_server()
@@ -214,5 +215,3 @@ class ShareServerTableTestCase(test.TestCase):
         db.share_server_delete(self.ctxt, server['id'])
         self.assertEqual(len(db.share_server_get_all(self.ctxt)),
                          num_records - 1)
-        self.assertFalse(
-            db.share_server_backend_details_get(self.ctxt, server['id']))