Avoid double-hopping deletes for security group rules

There is no need to get and delete; we can delete with one bullet.
This will most likely have quite a decent performance benefit overall.

The patch preserves the existing logic of raising and error on the missing
element; a test was added to spur up the coverage.

Related-bug: #1444112

Change-Id: Iaef77bd3f7775ed91d374838fb5488d925b4062c

diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py
index f14ed9cc515bc67b0cf3e058d82bc8eac30e6410..601279ccadb48242f5e9dc64d4570382cbf216ca 100644 (file)
--- a/neutron/db/securitygroups_db.py
+++ b/neutron/db/securitygroups_db.py
@@ -495,8 +495,9 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
 
     def delete_security_group_rule(self, context, id):
         with context.session.begin(subtransactions=True):
-            rule = self._get_security_group_rule(context, id)
-            context.session.delete(rule)
+            query = self._model_query(context, SecurityGroupRule)
+            if query.filter(SecurityGroupRule.id == id).delete() == 0:
+                raise ext_sg.SecurityGroupRuleNotFound(id=id)
 
     def _extend_port_dict_security_group(self, port_res, port_db):
         # Security group bindings will be retrieved from the sqlalchemy

diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py
new file mode 100644 (file)
index 0000000..693668c
--- /dev/null
+++ b/neutron/tests/unit/db/test_securitygroups_db.py
@@ -0,0 +1,38 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import testtools
+
+from neutron import context
+from neutron.db import common_db_mixin
+from neutron.db import securitygroups_db
+from neutron.extensions import securitygroup
+from neutron.tests.unit import testlib_api
+
+
+class SecurityGroupDbMixinImpl(securitygroups_db.SecurityGroupDbMixin,
+                               common_db_mixin.CommonDbMixin):
+    pass
+
+
+class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase):
+
+    def setUp(self):
+        super(SecurityGroupDbMixinTestCase, self).setUp()
+        self.ctx = context.get_admin_context()
+        self.mixin = SecurityGroupDbMixinImpl()
+
+    def test_delete_security_group_rule_raise_error_on_not_found(self):
+        with testtools.ExpectedException(
+            securitygroup.SecurityGroupRuleNotFound):
+            self.mixin.delete_security_group_rule(self.ctx, 'foo_rule')