Make each possible diff hunk its own class, rather than having them all be in

the DiffHunk class with a type tag. Now output is handled with polymorphism rather than dispatching with if/else. It also better shows what members each hunk type has.
2026-02-01 02:59:01 +00:00 · 2009-03-13 00:17:48 +00:00
parent fa9a63bf0a
commit 91956287c2
2 changed files with 165 additions and 124 deletions
--- a/ndiff/ndiff
+++ b/ndiff/ndiff
@@ -177,98 +177,153 @@ class Scan(object):
        finally:
            f.close()

-# The types of each possible diff hunk.
-(HOST_STATE_CHANGE, HOST_ADDRESS_ADD, HOST_ADDRESS_REMOVE,
-    HOST_HOSTNAME_ADD, HOST_HOSTNAME_REMOVE,
-    PORT_ID_CHANGE, PORT_STATE_CHANGE) = range(0, 7)

+# What follows are all the possible diff hunk types. Each is capable of redering
+# itself to a string or to an XML DOM fragment.
 class DiffHunk(object):
    """A DiffHunk is a single unit of a diff. Each one represents one atomic
    change: a host state change, port state change, and so on."""
-    def __init__(self, type):
-        self.type = type

    def to_string(self):
        """Return a human-readable string representation of this hunk."""
-        if self.type == HOST_STATE_CHANGE:
-            return u"Host is %s, was %s." % (self.b_state, self.a_state)
-        elif self.type == HOST_ADDRESS_ADD:
-            return u"Add %s address %s." % (self.address_type, self.address)
-        elif self.type == HOST_ADDRESS_REMOVE:
-            return u"Remove %s address %s." % (self.address_type, self.address)
-        elif self.type == HOST_HOSTNAME_ADD:
-            return u"Add hostname %s." % self.hostname
-        elif self.type == HOST_HOSTNAME_REMOVE:
-            return u"Remove hostname %s." % self.hostname
-        elif self.type == PORT_ID_CHANGE:
-            return u"Service on %s is now running on %s." % (Port.spec_to_string(self.a_spec), Port.spec_to_string(self.b_spec))
-        elif self.type == PORT_STATE_CHANGE:
-            if self.a_state == Port.UNKNOWN:
-                return u"%s is %s." % (Port.spec_to_string(self.spec), self.b_state)
-            else:
-                return u"%s is %s, was %s." % (Port.spec_to_string(self.spec), self.b_state, self.a_state)
-        else:
-            assert False
+        raise Exception(u"The to_string method must be overridden.")

    def to_dom_fragment(self, document):
        """Return an XML DocumentFragment representation of this hunk."""
+        raise Exception(u"The to_dom_fragment method must be overridden.")
+
+class HostStateChangeHunk(DiffHunk):
+    def __init__(self, a_state, b_state):
+        self.a_state = a_state
+        self.b_state = b_state
+
+    def to_string(self):
+        return u"Host is %s, was %s." % (self.b_state, self.a_state)
+
+    def to_dom_fragment(self, document):
        frag = document.createDocumentFragment()
-        if self.type == HOST_STATE_CHANGE:
-            elem = document.createElement(u"host-state-change")
-            elem.setAttribute(u"a-state", self.a_state)
-            elem.setAttribute(u"b-state", self.b_state)
-            frag.appendChild(elem)
-        elif self.type == HOST_ADDRESS_ADD:
-            elem = document.createElement(u"host-address-add")
-            frag.appendChild(elem)
-            address_elem = document.createElement("address")
-            address_elem.setAttribute(u"addrtype", self.address_type)
-            address_elem.setAttribute(u"addr", self.address)
-            elem.appendChild(address_elem)
-        elif self.type == HOST_ADDRESS_REMOVE:
-            elem = document.createElement(u"host-address-remove")
-            frag.appendChild(elem)
-            address_elem = document.createElement("address")
-            address_elem.setAttribute(u"addrtype", self.address_type)
-            address_elem.setAttribute(u"addr", self.address)
-            elem.appendChild(address_elem)
-        elif self.type == HOST_HOSTNAME_ADD:
-            elem = document.createElement(u"host-hostname-add")
-            frag.appendChild(elem)
-            address_elem = document.createElement("hostname")
-            address_elem.setAttribute(u"name", self.hostname)
-            elem.appendChild(address_elem)
-        elif self.type == HOST_HOSTNAME_REMOVE:
-            elem = document.createElement(u"host-hostname-remove")
-            frag.appendChild(elem)
-            address_elem = document.createElement("hostname")
-            address_elem.setAttribute(u"name", self.hostname)
-            elem.appendChild(address_elem)
-        elif self.type == PORT_ID_CHANGE:
-            elem = document.createElement(u"port-id-change")
-            elem.setAttribute(u"a-portid", unicode(self.a_spec[0]))
-            elem.setAttribute(u"a-protocol", self.a_spec[1])
-            elem.setAttribute(u"b-portid", unicode(self.b_spec[0]))
-            elem.setAttribute(u"b-protocol", self.b_spec[1])
-            frag.appendChild(elem)
-        elif self.type == PORT_STATE_CHANGE:
-            elem = document.createElement(u"port-state-change")
-            elem.setAttribute(u"portid", unicode(self.spec[0]))
-            elem.setAttribute(u"protocol", self.spec[1])
-            elem.setAttribute(u"a-state", self.a_state)
-            elem.setAttribute(u"b-state", self.b_state)
-            frag.appendChild(elem)
-        else:
-            assert False
+        elem = document.createElement(u"host-state-change")
+        elem.setAttribute(u"a-state", self.a_state)
+        elem.setAttribute(u"b-state", self.b_state)
+        frag.appendChild(elem)
        return frag

+class HostAddressAddHunk(DiffHunk):
+    def __init__(self, address_type, address):
+        self.address_type = address_type
+        self.address = address
+
+    def to_string(self):
+        return u"Add %s address %s." % (self.address_type, self.address)
+
+    def to_dom_fragment(self, document):
+        frag = document.createDocumentFragment()
+        elem = document.createElement(u"host-address-add")
+        frag.appendChild(elem)
+        address_elem = document.createElement(u"address")
+        address_elem.setAttribute(u"addrtype", self.address_type)
+        address_elem.setAttribute(u"addr", self.address)
+        elem.appendChild(address_elem)
+        return frag
+
+class HostAddressRemoveHunk(DiffHunk):
+    def __init__(self, address_type, address):
+        self.address_type = address_type
+        self.address = address
+
+    def to_string(self):
+        return u"Remove %s address %s." % (self.address_type, self.address)
+
+    def to_dom_fragment(self, document):
+        frag = document.createDocumentFragment()
+        elem = document.createElement(u"host-address-remove")
+        frag.appendChild(elem)
+        address_elem = document.createElement(u"address")
+        address_elem.setAttribute(u"addrtype", self.address_type)
+        address_elem.setAttribute(u"addr", self.address)
+        elem.appendChild(address_elem)
+        return frag
+
+class HostHostnameAddHunk(DiffHunk):
+    def __init__(self, hostname):
+        self.hostname = hostname
+
+    def to_string(self):
+        return u"Add hostname %s." % self.hostname
+
+    def to_dom_fragment(self, document):
+        frag = document.createDocumentFragment()
+        elem = document.createElement(u"host-hostname-add")
+        frag.appendChild(elem)
+        address_elem = document.createElement(u"hostname")
+        address_elem.setAttribute(u"name", self.hostname)
+        elem.appendChild(address_elem)
+        return frag
+
+class HostHostnameRemoveHunk(DiffHunk):
+    def __init__(self, hostname):
+        self.hostname = hostname
+
+    def to_string(self):
+        return u"Remove hostname %s." % self.hostname
+
+    def to_dom_fragment(self, document):
+        frag = document.createDocumentFragment()
+        elem = document.createElement(u"host-hostname-remove")
+        frag.appendChild(elem)
+        address_elem = document.createElement("hostname")
+        address_elem.setAttribute(u"name", self.hostname)
+        elem.appendChild(address_elem)
+        return frag
+
+class PortIdChangeHunk(DiffHunk):
+    def __init__(self, a_spec, b_spec):
+        self.a_spec = a_spec
+        self.b_spec = b_spec
+
+    def to_string(self):
+        return u"Service on %s is now running on %s." % (Port.spec_to_string(self.a_spec), Port.spec_to_string(self.b_spec))
+
+    def to_dom_fragment(self, document):
+        frag = document.createDocumentFragment()
+        elem = document.createElement(u"port-id-change")
+        elem.setAttribute(u"a-portid", unicode(self.a_spec[0]))
+        elem.setAttribute(u"a-protocol", self.a_spec[1])
+        elem.setAttribute(u"b-portid", unicode(self.b_spec[0]))
+        elem.setAttribute(u"b-protocol", self.b_spec[1])
+        frag.appendChild(elem)
+        return frag
+
+class PortStateChangeHunk(DiffHunk):
+    def __init__(self, spec, a_state, b_state):
+        self.spec = spec
+        self.a_state = a_state
+        self.b_state = b_state
+
+    def to_string(self):
+        if self.a_state == Port.UNKNOWN:
+            return u"%s is %s." % (Port.spec_to_string(self.spec), self.b_state)
+        else:
+            return u"%s is %s, was %s." % (Port.spec_to_string(self.spec), self.b_state, self.a_state)
+
+    def to_dom_fragment(self, document):
+        frag = document.createDocumentFragment()
+        elem = document.createElement(u"port-state-change")
+        elem.setAttribute(u"portid", unicode(self.spec[0]))
+        elem.setAttribute(u"protocol", self.spec[1])
+        elem.setAttribute(u"a-state", self.a_state)
+        elem.setAttribute(u"b-state", self.b_state)
+        frag.appendChild(elem)
+        return frag
+
+
 def partition_port_state_changes(diff):
-    """Partition a list of PORT_STATE_CHANGE diff hunks into equivalence classes
+    """Partition a list of PortStateChangeHunks into equivalence classes
    based on the tuple (protocol, a_state, b_state). The partition is returned
    as a list of lists of hunks."""
    transitions = {}
    for hunk in diff:
-        if hunk.type != PORT_STATE_CHANGE:
+        if not isinstance(hunk, PortStateChangeHunk):
            continue
        a_state = hunk.a_state
        b_state = hunk.b_state
@@ -277,7 +332,7 @@ def partition_port_state_changes(diff):
    return transitions.values()

 def consolidate_port_state_changes(diff, threshold = 0):
-    """Return a list of list of PORT_STATE_CHANGE diff hunks, where each list
+    """Return a list of list of PortStateChangeHunks, where each list
    contains hunks with the same partition and state change. A group of hunks is
    returned in the list of lists only when its length exceeds threshold. Any
    hunks moved to the list of lists are removed from diff in place. This is to
@@ -371,15 +426,10 @@ def port_diff(a, b):
    """Diff two Ports. The return value is a list of DiffHunks."""
    diff = []
    if a.spec != b.spec:
-        hunk = DiffHunk(PORT_ID_CHANGE)
-        hunk.a_spec = a.spec
-        hunk.b_spec = b.spec
+        hunk = PortIdChangeHunk(a.spec, b.spec)
        diff.append(hunk)
    if a.state != b.state:
-        hunk = DiffHunk(PORT_STATE_CHANGE)
-        hunk.spec = b.spec
-        hunk.a_state = a.state
-        hunk.b_state = b.state
+        hunk = PortStateChangeHunk(b.spec, a.state, b.state)
        diff.append(hunk)
    return diff

@@ -389,14 +439,10 @@ def addresses_diff(a, b):
    a_addresses = set(a)
    b_addresses = set(b)
    for addrtype, addr in a_addresses - b_addresses:
-        hunk = DiffHunk(HOST_ADDRESS_REMOVE)
-        hunk.address_type = addrtype
-        hunk.address = addr
+        hunk = HostAddressRemoveHunk(addrtype, addr)
        diff.append(hunk)
    for addrtype, addr in b_addresses - a_addresses:
-        hunk = DiffHunk(HOST_ADDRESS_ADD)
-        hunk.address_type = addrtype
-        hunk.address = addr
+        hunk = HostAddressAddHunk(addrtype, addr)
        diff.append(hunk)
    return diff

@@ -407,12 +453,10 @@ def hostnames_diff(a, b):
    b_hostnames = set(b)

    for hostname in a_hostnames - b_hostnames:
-        hunk = DiffHunk(HOST_HOSTNAME_REMOVE)
-        hunk.hostname = hostname
+        hunk = HostHostnameRemoveHunk(hostname)
        diff.append(hunk)
    for hostname in b_hostnames - a_hostnames:
-        hunk = DiffHunk(HOST_HOSTNAME_ADD)
-        hunk.hostname = hostname
+        hunk = HostHostnameAddHunk(hostname)
        diff.append(hunk)
    return diff

@@ -420,10 +464,7 @@ def host_diff(a, b):
    """Diff two Hosts. The return value is a list of DiffHunks."""
    diff = []
    if a.state != b.state:
-        hunk = DiffHunk(HOST_STATE_CHANGE)
-        hunk.id = a.get_id()
-        hunk.a_state = a.state
-        hunk.b_state = b.state
+        hunk = HostStateChangeHunk(a.state, b.state)
        diff.append(hunk)

    diff.extend(addresses_diff(a.addresses, b.addresses))
--- a/ndiff/ndifftest.py
+++ b/ndiff/ndifftest.py
@@ -71,7 +71,7 @@ class partition_port_state_changes_test(unittest.TestCase):
            partition = partition_port_state_changes(h_diff)
            for group in partition:
                for hunk in group:
-                    self.assertTrue(hunk.type == PORT_STATE_CHANGE)
+                    self.assertTrue(isinstance(hunk, PortStateChangeHunk))

    def test_equivalence(self):
        for host, h_diff in self.diff:
@@ -94,7 +94,7 @@ class consolidate_port_state_changes_test(unittest.TestCase):
        for host, h_diff in self.diff:
            consolidated = consolidate_port_state_changes(h_diff, 0)
            for hunk in h_diff:
-                self.assertTrue(hunk.type != PORT_STATE_CHANGE)
+                self.assertTrue(not isinstance(hunk, PortStateChangeHunk))

    def test_conservation(self):
        pre_length = 0
@@ -131,7 +131,7 @@ class port_diff_test(unittest.TestCase):
        b = Port((20, "tcp"))
        diff = port_diff(a, b)
        self.assertTrue(len(diff) == 1)
-        self.assertTrue(diff[0].type == PORT_ID_CHANGE)
+        self.assertTrue(isinstance(diff[0], PortIdChangeHunk))

    def test_state_change(self):
        spec = (10, "tcp")
@@ -141,7 +141,7 @@ class port_diff_test(unittest.TestCase):
        b.state = "closed"
        diff = port_diff(a, b)
        self.assertTrue(len(diff) == 1)
-        self.assertTrue(diff[0].type == PORT_STATE_CHANGE)
+        self.assertTrue(isinstance(diff[0], PortStateChangeHunk))

    def test_id_state_change(self):
        a = Port((10, "tcp"))
@@ -212,20 +212,20 @@ class host_test(unittest.TestCase):
 def host_apply_diff(host, diff):
    """Apply a host diff to the given host."""
    for hunk in diff:
-        if hunk.type == HOST_STATE_CHANGE:
+        if isinstance(hunk, HostStateChangeHunk):
            assert host.state == hunk.a_state
            host.state = hunk.b_state
-        elif hunk.type == HOST_ADDRESS_ADD:
+        elif isinstance(hunk, HostAddressAddHunk):
            host.add_address(hunk.address_type, hunk.address)
-        elif hunk.type == HOST_ADDRESS_REMOVE:
+        elif isinstance(hunk, HostAddressRemoveHunk):
            host.remove_address(hunk.address_type, hunk.address)
-        elif hunk.type == HOST_HOSTNAME_ADD:
+        elif isinstance(hunk, HostHostnameAddHunk):
            host.add_hostname(hunk.hostname)
-        elif hunk.type == HOST_HOSTNAME_REMOVE:
+        elif isinstance(hunk, HostHostnameRemoveHunk):
            host.remove_hostname(hunk.hostname)
-        elif hunk.type == PORT_ID_CHANGE:
+        elif isinstance(hunk, PortIdChangeHunk):
            host.swap_ports(hunk.a_spec, hunk.b_spec)
-        elif hunk.type == PORT_STATE_CHANGE:
+        elif isinstance(hunk, PortStateChangeHunk):
            port = host.ports[hunk.spec]
            assert port.state == hunk.a_state
            host.add_port(hunk.spec, hunk.b_state)
@@ -234,8 +234,8 @@ def host_apply_diff(host, diff):

 class host_diff_test(unittest.TestCase):
    """Test the host_diff function."""
-    PORT_DIFF_HUNK_TYPES = (PORT_ID_CHANGE, PORT_STATE_CHANGE)
-    HOST_DIFF_HUNK_TYPES = (HOST_STATE_CHANGE,) + PORT_DIFF_HUNK_TYPES
+    PORT_DIFF_HUNK_TYPES = (PortIdChangeHunk, PortStateChangeHunk)
+    HOST_DIFF_HUNK_TYPES = (HostStateChangeHunk,) + PORT_DIFF_HUNK_TYPES

    def test_empty(self):
        a = Host()
@@ -258,7 +258,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in self.HOST_DIFF_HUNK_TYPES)
+            self.assertTrue(isinstance(hunk, self.HOST_DIFF_HUNK_TYPES))

    def test_state_change_unknown(self):
        a = Host()
@@ -267,11 +267,11 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in self.HOST_DIFF_HUNK_TYPES)
+            self.assertTrue(isinstance(hunk, self.HOST_DIFF_HUNK_TYPES))
        diff = host_diff(b, a)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in self.HOST_DIFF_HUNK_TYPES)
+            self.assertTrue(isinstance(hunk, self.HOST_DIFF_HUNK_TYPES))

    def test_port_state_change(self):
        a = Host()
@@ -282,7 +282,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in self.PORT_DIFF_HUNK_TYPES)
+            self.assertTrue(isinstance(hunk, self.PORT_DIFF_HUNK_TYPES))

    def test_port_state_change_unknown(self):
        a = Host()
@@ -291,11 +291,11 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in self.PORT_DIFF_HUNK_TYPES)
+            self.assertTrue(isinstance(hunk, self.PORT_DIFF_HUNK_TYPES))
        diff = host_diff(b, a)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in self.PORT_DIFF_HUNK_TYPES)
+            self.assertTrue(isinstance(hunk, self.PORT_DIFF_HUNK_TYPES))

    def test_port_state_change_multi(self):
        a = Host()
@@ -309,7 +309,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in self.PORT_DIFF_HUNK_TYPES)
+            self.assertTrue(isinstance(hunk, self.PORT_DIFF_HUNK_TYPES))

    def test_address_add(self):
        a = Host()
@@ -319,7 +319,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type == HOST_ADDRESS_ADD)
+            self.assertTrue(isinstance(hunk, HostAddressAddHunk))

    def test_address_add(self):
        a = Host()
@@ -329,7 +329,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type == HOST_ADDRESS_REMOVE)
+            self.assertTrue(isinstance(hunk, HostAddressRemoveHunk))

    def test_address_add(self):
        a = Host()
@@ -339,7 +339,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in (HOST_ADDRESS_ADD, HOST_ADDRESS_REMOVE))
+            self.assertTrue(isinstance(hunk, (HostAddressAddHunk, HostAddressRemoveHunk)))

    def test_hostname_add(self):
        a = Host()
@@ -349,7 +349,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type == HOST_HOSTNAME_ADD)
+            self.assertTrue(isinstance(hunk, HostHostnameAddHunk))

    def test_hostname_remove(self):
        a = Host()
@@ -359,7 +359,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type == HOST_HOSTNAME_REMOVE)
+            self.assertTrue(isinstance(hunk, HostHostnameRemoveHunk))

    def test_hostname_change(self):
        a = Host()
@@ -369,7 +369,7 @@ class host_diff_test(unittest.TestCase):
        diff = host_diff(a, b)
        self.assertTrue(len(diff) > 0)
        for hunk in diff:
-            self.assertTrue(hunk.type in (HOST_HOSTNAME_ADD, HOST_HOSTNAME_REMOVE))
+            self.assertTrue(isinstance(hunk, (HostHostnameAddHunk, HostHostnameRemoveHunk)))

    def test_diff_is_effective(self):
        """Test that a host diff is effective.
@@ -486,7 +486,7 @@ class scan_diff_test(unittest.TestCase):
        diff = scan_diff(a, b)
        for host, h_diff in diff:
            for hunk in h_diff:
-                if hunk.type == HOST_STATE_CHANGE:
+                if isinstance(hunk, HostStateChangeHunk):
                    self.assertTrue(hunk.a_state == Host.UNKNOWN)
                    self.assertTrue(hunk.b_state == u"up")
                    break
@@ -501,7 +501,7 @@ class scan_diff_test(unittest.TestCase):
        diff = scan_diff(a, b)
        for host, h_diff in diff:
            for hunk in h_diff:
-                if hunk.type == HOST_STATE_CHANGE:
+                if isinstance(hunk, HostStateChangeHunk):
                    self.assertTrue(hunk.a_state == u"up")
                    self.assertTrue(hunk.b_state == Port.UNKNOWN)
                    break