Author: aconway
Date: Fri Dec  5 13:42:53 2014
New Revision: 1643280

URL: http://svn.apache.org/r1643280
Log:
DISPATCH-79: Fix ordering bug in agent.py code.

There was an ordering bug in the agent code. It was possible for an entity to be
removed, then another entity with the same name/identity added, but the agent
would evaluate the "add" before the "remove" and complain about a duplicate
object.

Modified:
    qpid/dispatch/trunk/python/qpid_dispatch_internal/management/agent.py
    qpid/dispatch/trunk/tests/system_tests_management.py

Modified: qpid/dispatch/trunk/python/qpid_dispatch_internal/management/agent.py
URL: 
http://svn.apache.org/viewvc/qpid/dispatch/trunk/python/qpid_dispatch_internal/management/agent.py?rev=1643280&r1=1643279&r2=1643280&view=diff
==============================================================================
--- qpid/dispatch/trunk/python/qpid_dispatch_internal/management/agent.py 
(original)
+++ qpid/dispatch/trunk/python/qpid_dispatch_internal/management/agent.py Fri 
Dec  5 13:42:53 2014
@@ -113,8 +113,7 @@ class Entity(SchemaEntity):
 
     def _set_pointer(self, pointer):
         fname = "qd_c_entity_refresh_" + 
self.entity_type.short_name.replace('.', '_')
-        refreshfn = self._qd.function(
-            fname, c_long, [py_object, c_void_p])
+        refreshfn = self._qd.function(fname, c_long, [py_object, c_void_p])
         def _do_refresh():
             refreshfn(self.attributes, pointer)
             return True
@@ -307,49 +306,41 @@ class EntityCache(object):
             del self.pointers[pointer]
             self._remove(entity)
 
+
     def refresh_from_c(self):
         """Refresh entities from the C dispatch runtime"""
-        events = []
-        REMOVE, ADD, REMOVE_ADD = 0, 1, 2
-
-        class Action(object):
-            """Collapse a sequence of add/remove actions down to None, remove, 
add or remove_add"""
-
-            MATRIX = {          # Collaps pairs of actions
-                (None, ADD): ADD,
-                (None, REMOVE): REMOVE,
-                (REMOVE, ADD): REMOVE_ADD,
-                (ADD, REMOVE): None,
-                (REMOVE_ADD, REMOVE): REMOVE
-            }
-
-            def __init__(self, type):
-                self.action = None
-                self.type = type
-
-            def add(self, action):
-                try: self.action = self.MATRIX[(self.action, action)]
-                except KeyError: pass
+        REMOVE, ADD = 0, 1
 
+        def remove_redundant(events):
+            """Remove redundant add/remove pairs of events."""
+            add = {}            # add[pointer] = index of add event.
+            redundant = []      # List of redundant event indexes.
+            for i in xrange(len(events)):
+                action, type, pointer = events[i]
+                if action == ADD:
+                    add[pointer] = i
+                elif pointer in add: # action == REMOVE and there's an ADD
+                    redundant.append(add[pointer])
+                    redundant.append(i)
+                    del add[pointer]
+            for i in sorted(redundant, reverse=True):
+                events.pop(i)
 
         # FIXME aconway 2014-10-23: locking is ugly, push it down into C code.
         self.qd.qd_dispatch_router_lock(self.agent.dispatch)
         try:
+            events = []
             self.qd.qd_c_entity_refresh_begin(events)
-            # Collapse sequences of add/remove into a single 
remove/add/remove_add per pointer.
-            actions = {}
+            remove_redundant(events)
             for action, type, pointer in events:
-                if not pointer in actions: actions[pointer] = Action(type)
-                actions[pointer].add(action)
-            for pointer, action in actions.iteritems():
-                if action.action == REMOVE or action.action == REMOVE_ADD:
+                if action == REMOVE:
                     self._remove_pointer(pointer)
-                if action.action == ADD or action.action == REMOVE_ADD:
-                    entity_type = self.schema.entity_type(action.type)
+                elif action == ADD:
+                    entity_type = self.schema.entity_type(type)
                     klass = self.agent.entity_class(entity_type)
                     entity = klass(self.agent, entity_type, pointer)
                     self.add(entity, pointer)
-
+            # Refresh the entity values while the lock is still held.
             for e in self.entities: e._refresh()
         finally:
             self.qd.qd_c_entity_refresh_end()

Modified: qpid/dispatch/trunk/tests/system_tests_management.py
URL: 
http://svn.apache.org/viewvc/qpid/dispatch/trunk/tests/system_tests_management.py?rev=1643280&r1=1643279&r2=1643280&view=diff
==============================================================================
--- qpid/dispatch/trunk/tests/system_tests_management.py (original)
+++ qpid/dispatch/trunk/tests/system_tests_management.py Fri Dec  5 13:42:53 
2014
@@ -149,12 +149,12 @@ class ManagementTest(system_test.TestCas
                             msg="Can't find expected ValidationError.*%s in 
'%r'" % (bad_type, logstr))
 
         # Create a log entity, verify logging is as expected
-        log = os.path.abspath("test_create_log.log")
-        agent_log = self.assert_create_ok('log', 'log.1', dict(module='AGENT', 
level="error", output=log))
+        log = os.path.abspath("test_log.log")
+        agent_log = self.assert_create_ok('log', 'log:AGENT', 
dict(module='AGENT', level="error", output=log))
         check_log(log)
 
         # Update the log entity to output to a different file
-        log = os.path.abspath("test_create_log2.log")
+        log = os.path.abspath("test_log2.log")
         self.node.update(dict(module='AGENT', level="error", output=log), 
identity='log:AGENT')
         check_log(log)
 
@@ -162,6 +162,13 @@ class ManagementTest(system_test.TestCas
         self.node.delete(identity='log:AGENT')
         self.assertRaises(AssertionError, check_log, log, 'nosuch2')
 
+        # Verify that debug logging shows up if enabled for a module
+        self.node.create(dict(module='AGENT', level="debug", output=log), 
type='log', name="log:AGENT")
+        logstr = self.cleanup(open(log)).read()
+        self.assertTrue(re.search(r'AGENT \(debug\)', logstr),
+                        msg="Can't find 'AGENT (debug)' messages in '%s'" % 
(logstr))
+
+
     def test_create_fixed_address(self):
         self.assert_create_ok(FIXED_ADDRESS, 'fixed1', dict(prefix='fixed1'))
         msgr = self.messenger()



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to