Merge "Throw HTTP 409 (Conflict) on duplicate rules"

congress/api/application.py

@@ -51,7 +51,8 @@ class ApiApplication(object):
                 response = NOT_FOUND_RESPONSE
         except DataModelException as e:
             # Error raised based on invalid user input
-            return e.rest_response()
+            LOG.debug("ApiApplication: found DataModelException " + str(e))
+            response = e.rest_response()
         except Exception as e:
             # Unexpected error raised by API framework or data model
             msg = _("Exception caught for request: %s")

congress/api/error_codes.py

@@ -26,6 +26,8 @@ errors['sequence_syntax'] = (
     1006, "Syntax error in sequence")
 errors['simulate_error'] = (
     1007, "Error in simulate procedure")
+errors['rule_already_exists'] = (
+    1008, "Rule already exists")
 
 
 def get(name):

congress/api/rule_model.py

@@ -12,13 +12,19 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 #
+import httplib
+
 from congress.api import error_codes
 from congress.api import webservice
 from congress.dse import deepsix
+from congress.openstack.common import log as logging
 from congress.policy import compile
 from congress.policy import runtime
 
 
+LOG = logging.getLogger(__name__)
+
+
 def d6service(name, keys, inbox, datapath, args):
     return RuleModel(name, keys, inbox=inbox, dataPath=datapath, **args)
 
@@ -105,6 +111,7 @@ class RuleModel(deepsix.deepSix):
         """
         # TODO(thinrichs): add comment property to rule
         if id_ is not None:
+            LOG.debug("add_item error: should not be given ID")
             raise webservice.DataModelException(
                 **error_codes.get('add_item_id'))
         str_rule = item['rule']
@@ -113,12 +120,14 @@ class RuleModel(deepsix.deepSix):
             if len(rule) == 1:
                 rule = rule[0]
             else:
+                LOG.debug("add_item error: given too many rules")
                 (num, desc) = error_codes.get('multiple_rules')
                 raise webservice.DataModelException(
                     num, desc + ":: Received multiple rules: " +
                     "; ".join(str(x) for x in rule))
             changes = self.change_rule(rule, context)
         except compile.CongressException as e:
+            LOG.debug("add_item error: invalid rule syntax")
             (num, desc) = error_codes.get('rule_syntax')
             raise webservice.DataModelException(num, desc + "::" + str(e))
 
@@ -128,15 +137,10 @@ class RuleModel(deepsix.deepSix):
                      'id': rule.id,
                      'comment': None}
                 return (rule.id, d)
-        # rule already existed
-        policy_name = self.policy_name(context)
-        for p in self.engine.theory[policy_name].policy():
-            if p == rule:
-                d = {'rule': rule.pretty_str(),
-                     'id': rule.id,
-                     'comment': 'None'}
-                return (rule.id, d)
-        raise Exception("add_item added a rule but then could not find it.")
+
+        num, desc = error_codes.get('rule_already_exists')
+        raise webservice.DataModelException(
+            num, desc, http_status_code=httplib.CONFLICT)
 
     def delete_item(self, id_, params, context=None):
         """Remove item from model.

congress/tests/test_congress.py

@@ -318,6 +318,15 @@ class TestCongress(unittest.TestCase):
             api['rule'].add_item(
                 {'rule': 'r(x) :- q(x), not p(x)'}, {}, context=context)
 
+        # duplicate rules
+        api['rule'].add_item(
+            {'rule': 'p(x) :- q(x)'}, {}, context=context)
+        with self.assertRaises(
+            webservice.DataModelException,
+            msg="Duplicate rule error not properly thrown"):
+            api['rule'].add_item(
+                {'rule': 'p(x) :- q(x)'}, {}, context=context)
+
     def test_table_api_model(self):
         """Test the table api model."""
         api = self.api