From f521d509db2b38051dbef5e2000d145d37b71672 Mon Sep 17 00:00:00 2001
From: Sumit Naiksatam <sumitnaiksatam@gmail.com>
Date: Tue, 17 Sep 2013 23:27:20 -0700
Subject: [PATCH] FWaaS - fix reordering of rules in policy

Due to a recent change, reodering of rules within the
same policy was failing. This is fixed by checking if
the rules belong to the same policy we allow reordering.

There was also a missing call to reorder due to which
the position number on the rules was not reflected correctly
after the reordering. This is also fixed.

Closes bug: #1226941

Change-Id: I7f52e8b9d578c290ace3bb615bf68bd213398303
---
 neutron/db/firewall/firewall_db.py            |  7 ++-
 .../unit/db/firewall/test_db_firewall.py      | 48 +++++++++++++++++++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/neutron/db/firewall/firewall_db.py b/neutron/db/firewall/firewall_db.py
index 0aa419204..b57475877 100644
--- a/neutron/db/firewall/firewall_db.py
+++ b/neutron/db/firewall/firewall_db.py
@@ -184,8 +184,10 @@ class Firewall_db_mixin(firewall.FirewallPluginBase, base_db.CommonDbMixin):
                     raise firewall.FirewallRuleNotFound(firewall_rule_id=
                                                         fwrule_id)
                 elif rules_dict[fwrule_id]['firewall_policy_id']:
-                    raise firewall.FirewallRuleInUse(
-                        firewall_rule_id=fwrule_id)
+                    if (rules_dict[fwrule_id]['firewall_policy_id'] !=
+                            fwp_db['id']):
+                        raise firewall.FirewallRuleInUse(
+                            firewall_rule_id=fwrule_id)
             # New list of rules is valid so we will first reset the existing
             # list and then add each rule in order.
             # Note that the list could be empty in which case we interpret
@@ -193,6 +195,7 @@ class Firewall_db_mixin(firewall.FirewallPluginBase, base_db.CommonDbMixin):
             fwp_db.firewall_rules = []
             for fwrule_id in rule_id_list:
                 fwp_db.firewall_rules.append(rules_dict[fwrule_id])
+            fwp_db.firewall_rules.reorder()
             fwp_db.audited = False
 
     def _process_rule_for_policy(self, context, firewall_policy_id,
diff --git a/neutron/tests/unit/db/firewall/test_db_firewall.py b/neutron/tests/unit/db/firewall/test_db_firewall.py
index c2d32b62c..7632a8da4 100644
--- a/neutron/tests/unit/db/firewall/test_db_firewall.py
+++ b/neutron/tests/unit/db/firewall/test_db_firewall.py
@@ -420,6 +420,54 @@ class TestFirewallDBPlugin(FirewallPluginDbTestCase):
                 for k, v in attrs.iteritems():
                     self.assertEqual(res['firewall_policy'][k], v)
 
+    def test_update_firewall_policy_reorder_rules(self):
+        attrs = self._get_test_firewall_policy_attrs()
+
+        with self.firewall_policy() as fwp:
+            with contextlib.nested(self.firewall_rule(name='fwr1',
+                                                      no_delete=True),
+                                   self.firewall_rule(name='fwr2',
+                                                      no_delete=True),
+                                   self.firewall_rule(name='fwr3',
+                                                      no_delete=True),
+                                   self.firewall_rule(name='fwr4',
+                                                      no_delete=True)) as fr:
+                fw_rule_ids = [fr[2]['firewall_rule']['id'],
+                               fr[3]['firewall_rule']['id']]
+                data = {'firewall_policy':
+                        {'firewall_rules': fw_rule_ids}}
+                req = self.new_update_request('firewall_policies', data,
+                                              fwp['firewall_policy']['id'])
+                req.get_response(self.ext_api)
+                # shuffle the rules, add more rules
+                fw_rule_ids = [fr[1]['firewall_rule']['id'],
+                               fr[3]['firewall_rule']['id'],
+                               fr[2]['firewall_rule']['id'],
+                               fr[0]['firewall_rule']['id']]
+                attrs['firewall_rules'] = fw_rule_ids
+                data = {'firewall_policy':
+                        {'firewall_rules': fw_rule_ids}}
+                req = self.new_update_request('firewall_policies', data,
+                                              fwp['firewall_policy']['id'])
+                res = self.deserialize(self.fmt,
+                                       req.get_response(self.ext_api))
+                rules = []
+                for rule_id in fw_rule_ids:
+                    req = self.new_show_request('firewall_rules',
+                                                rule_id,
+                                                fmt=self.fmt)
+                    res = self.deserialize(self.fmt,
+                                           req.get_response(self.ext_api))
+                    rules.append(res['firewall_rule'])
+                self.assertEqual(rules[0]['position'], 1)
+                self.assertEqual(rules[0]['id'], fr[1]['firewall_rule']['id'])
+                self.assertEqual(rules[1]['position'], 2)
+                self.assertEqual(rules[1]['id'], fr[3]['firewall_rule']['id'])
+                self.assertEqual(rules[2]['position'], 3)
+                self.assertEqual(rules[2]['id'], fr[2]['firewall_rule']['id'])
+                self.assertEqual(rules[3]['position'], 4)
+                self.assertEqual(rules[3]['id'], fr[0]['firewall_rule']['id'])
+
     def test_update_firewall_policy_with_non_existing_rule(self):
         attrs = self._get_test_firewall_policy_attrs()
 
-- 
2.45.2