From: John Schwarz <jschwarz@redhat.com>
Date: Mon, 3 Aug 2015 15:33:44 +0000 (+0300)
Subject: Gracefully handle duplicate rule creation
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=75737c5ef0f7abe8aab80f77336ff9be18494ebc;p=openstack-build%2Fneutron-build.git

Gracefully handle duplicate rule creation

Previously, creating a second bandwidth limit rule for a policy raised
an uncaught exception, which eventually caused 'ServerFault' on the
client side. This patch replaces this exception with a NeutronException
which leads to a more correct 'Conflict' error instead.

Note that the code is implemented in the base object class. This means
that future versioned objects will also feature this restriction if
their database implies that no duplicate entries can be created.

Change-Id: I882d60843e1e651f3f9754746ac670f499431466
Partially-Implements: quantum-qos-api
---

diff --git a/neutron/objects/base.py b/neutron/objects/base.py
index 5339fce27..f10966106 100644
--- a/neutron/objects/base.py
+++ b/neutron/objects/base.py
@@ -12,6 +12,7 @@
 
 import abc
 
+from oslo_db import exception as obj_exc
 from oslo_versionedobjects import base as obj_base
 import six
 
@@ -23,6 +24,10 @@ class NeutronObjectUpdateForbidden(exceptions.NeutronException):
     message = _("Unable to update the following object fields: %(fields)s")
 
 
+class NeutronObjectDuplicateEntry(exceptions.Conflict):
+    message = _("Failed to create a duplicate object")
+
+
 def get_updatable_fields(cls, fields):
     fields = fields.copy()
     for field in cls.fields_no_update:
@@ -116,7 +121,10 @@ class NeutronDbObject(NeutronObject):
 
     def create(self):
         fields = self._get_changed_persistent_fields()
-        db_obj = db_api.create_object(self._context, self.db_model, fields)
+        try:
+            db_obj = db_api.create_object(self._context, self.db_model, fields)
+        except obj_exc.DBDuplicateEntry:
+            raise NeutronObjectDuplicateEntry()
         self.from_db_object(db_obj)
 
     def update(self):
diff --git a/neutron/tests/api/test_qos.py b/neutron/tests/api/test_qos.py
index 7a0f02766..453b85387 100644
--- a/neutron/tests/api/test_qos.py
+++ b/neutron/tests/api/test_qos.py
@@ -298,7 +298,7 @@ class QosBandwidthLimitRuleTestJSON(base.BaseAdminNetworkTest):
                                              max_kbps=200,
                                              max_burst_kbps=1337)
 
-        self.assertRaises(exceptions.ServerFault,
+        self.assertRaises(exceptions.Conflict,
                           self.create_qos_bandwidth_limit_rule,
                           policy_id=policy['id'],
                           max_kbps=201, max_burst_kbps=1338)
diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py
index 7f8be5b89..84bdb13be 100644
--- a/neutron/tests/unit/objects/test_base.py
+++ b/neutron/tests/unit/objects/test_base.py
@@ -14,6 +14,7 @@ import random
 import string
 
 import mock
+from oslo_db import exception as obj_exc
 from oslo_versionedobjects import base as obj_base
 from oslo_versionedobjects import fields as obj_fields
 
@@ -154,6 +155,12 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
             obj.create()
             self._check_equal(obj, self.db_obj)
 
+    def test_create_duplicates(self):
+        with mock.patch.object(db_api, 'create_object',
+                               side_effect=obj_exc.DBDuplicateEntry):
+            obj = self._test_class(self.context, **self.db_obj)
+            self.assertRaises(base.NeutronObjectDuplicateEntry, obj.create)
+
     @mock.patch.object(db_api, 'update_object')
     def test_update_no_changes(self, update_mock):
         with mock.patch.object(base.NeutronDbObject,