Merge "Avoid performing extra query for fetching mac learning binding"

neutron/plugins/nicira/NeutronPlugin.py

@@ -1228,8 +1228,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         with context.session.begin(subtransactions=True):
             neutron_lports = super(NvpPluginV2, self).get_ports(
                 context, filters)
-            for neutron_lport in neutron_lports:
-                self._extend_port_mac_learning_state(context, neutron_lport)
         if (filters.get('network_id') and len(filters.get('network_id')) and
             self._network_is_external(context, filters['network_id'][0])):
             # Do not perform check on NVP platform
@@ -1422,6 +1420,9 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         with context.session.begin(subtransactions=True):
             ret_port = super(NvpPluginV2, self).update_port(
                 context, id, port)
+            # Save current mac learning state to check whether it's
+            # being updated or not
+            old_mac_learning_state = ret_port.get(mac_ext.MAC_LEARNING)
             # copy values over - except fixed_ips as
             # they've alreaby been processed
             port['port'].pop('fixed_ips', None)
@@ -1459,15 +1460,11 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                 context, ret_port)
             # Populate the mac learning attribute
             new_mac_learning_state = port['port'].get(mac_ext.MAC_LEARNING)
-            old_mac_learning_state = self._get_mac_learning_state(context, id)
             if (new_mac_learning_state is not None and
                 old_mac_learning_state != new_mac_learning_state):
                 self._update_mac_learning_state(context, id,
                                                 new_mac_learning_state)
                 ret_port[mac_ext.MAC_LEARNING] = new_mac_learning_state
-            elif (new_mac_learning_state is None and
-                  old_mac_learning_state is not None):
-                ret_port[mac_ext.MAC_LEARNING] = old_mac_learning_state
             self._delete_port_queue_mapping(context, ret_port['id'])
             self._process_port_queue_mapping(context, ret_port)
             LOG.warn(_("Update port request: %s"), port)
@@ -1561,7 +1558,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
             neutron_db_port = super(NvpPluginV2, self).get_port(context,
                                                                 id, fields)
             self._extend_port_qos_queue(context, neutron_db_port)
-            self._extend_port_mac_learning_state(context, neutron_db_port)
 
             if self._network_is_external(context,
                                          neutron_db_port['network_id']):

neutron/plugins/nicira/dbexts/maclearning.py

@@ -16,9 +16,13 @@
 #
 
 import sqlalchemy as sa
+from sqlalchemy import orm
 from sqlalchemy.orm import exc
 
+from neutron.api.v2 import attributes
+from neutron.db import db_base_plugin_v2
 from neutron.db import model_base
+from neutron.db import models_v2
 from neutron.openstack.common import log as logging
 from neutron.plugins.nicira.extensions import maclearning as mac
 
@@ -32,6 +36,13 @@ class MacLearningState(model_base.BASEV2):
                         primary_key=True)
     mac_learning_enabled = sa.Column(sa.Boolean(), nullable=False)
 
+    # Add a relationship to the Port model using the backref attribute.
+    # This will instruct SQLAlchemy to eagerly load this association.
+    port = orm.relationship(
+        models_v2.Port,
+        backref=orm.backref("mac_learning_state", lazy='joined',
+                            uselist=False, cascade='delete'))
+
 
 class MacLearningDbMixin(object):
     """Mixin class for mac learning."""
@@ -41,18 +52,14 @@ class MacLearningDbMixin(object):
                mac.MAC_LEARNING: port[mac.MAC_LEARNING]}
         return self._fields(res, fields)
 
-    def _get_mac_learning_state(self, context, port_id):
+    def _extend_port_mac_learning_state(self, port_res, port_db):
-        try:
+        state = port_db.mac_learning_state
-            query = self._model_query(context, MacLearningState)
+        if state and state.mac_learning_enabled:
-            state = query.filter(MacLearningState.port_id == port_id).one()
+            port_res[mac.MAC_LEARNING] = state.mac_learning_enabled
-        except exc.NoResultFound:
-            return None
-        return state[mac.MAC_LEARNING]
 
-    def _extend_port_mac_learning_state(self, context, port):
+    # Register dict extend functions for ports
-        state = self._get_mac_learning_state(context, port['id'])
+    db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs(
-        if state:
+        attributes.PORTS, [_extend_port_mac_learning_state])
-            port[mac.MAC_LEARNING] = state
 
     def _update_mac_learning_state(self, context, port_id, enabled):
         try:

neutron/tests/unit/nicira/test_maclearning.py

@@ -91,15 +91,18 @@ class MacLearningDBTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
     def test_create_with_mac_learning(self):
         with self.port(arg_list=('mac_learning_enabled',),
                        mac_learning_enabled=True) as port:
+            # Validate create operation response
+            self.assertEqual(True, port['port']['mac_learning_enabled'])
+            # Verify that db operation successfully set mac learning state
             req = self.new_show_request('ports', port['port']['id'], self.fmt)
             sport = self.deserialize(self.fmt, req.get_response(self.api))
-            self.assertTrue(sport['port']['mac_learning_enabled'])
+            self.assertEqual(True, sport['port']['mac_learning_enabled'])
 
-    def test_create_port_without_mac_learning(self):
+    def test_create_and_show_port_without_mac_learning(self):
         with self.port() as port:
             req = self.new_show_request('ports', port['port']['id'], self.fmt)
             sport = self.deserialize(self.fmt, req.get_response(self.api))
-            self.assertNotIn('mac_learning', sport['port'])
+            self.assertNotIn('mac_learning_enabled', sport['port'])
 
     def test_update_port_with_mac_learning(self):
         with self.port(arg_list=('mac_learning_enabled',),
@@ -107,7 +110,7 @@ class MacLearningDBTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
             data = {'port': {'mac_learning_enabled': True}}
             req = self.new_update_request('ports', data, port['port']['id'])
             res = self.deserialize(self.fmt, req.get_response(self.api))
-            self.assertTrue(res['port']['mac_learning_enabled'])
+            self.assertEqual(True, res['port']['mac_learning_enabled'])
 
     def test_update_preexisting_port_with_mac_learning(self):
         with self.port() as port:
@@ -116,8 +119,13 @@ class MacLearningDBTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
             self.assertNotIn('mac_learning_enabled', sport['port'])
             data = {'port': {'mac_learning_enabled': True}}
             req = self.new_update_request('ports', data, port['port']['id'])
+            # Validate update operation response
             res = self.deserialize(self.fmt, req.get_response(self.api))
-            self.assertTrue(res['port']['mac_learning_enabled'])
+            self.assertEqual(True, res['port']['mac_learning_enabled'])
+            # Verify that db operation successfully updated mac learning state
+            req = self.new_show_request('ports', port['port']['id'], self.fmt)
+            sport = self.deserialize(self.fmt, req.get_response(self.api))
+            self.assertEqual(True, sport['port']['mac_learning_enabled'])
 
     def test_list_ports(self):
         # for this test we need to enable overlapping ips
@@ -129,4 +137,10 @@ class MacLearningDBTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
                                self.port(arg_list=('mac_learning_enabled',),
                                          mac_learning_enabled=True)):
             for port in self._list('ports')['ports']:
-                self.assertTrue(port['mac_learning_enabled'])
+                self.assertEqual(True, port['mac_learning_enabled'])
+
+    def test_show_port(self):
+        with self.port(arg_list=('mac_learning_enabled',),
+                       mac_learning_enabled=True) as p:
+            port_res = self._show('ports', p['port']['id'])['port']
+            self.assertEqual(True, port_res['mac_learning_enabled'])