From a29a7a558277e9438e288dbeaa1bbd6839bc7dcf Mon Sep 17 00:00:00 2001
From: Blake Harnden <32446120+bharnden@users.noreply.github.com>
Date: Tue, 16 Jun 2020 14:18:19 -0700
Subject: [PATCH] refactored LinkOptions to be used within LinkData, instead of
 duplicating data, removed session from LinkOptions and LinkData

---
 daemon/core/api/grpc/grpcutils.py   | 30 ++++++++++++----------
 daemon/core/api/tlv/corehandlers.py | 31 +++++++++++-----------
 daemon/core/emulator/data.py        | 25 ++++++------------
 daemon/core/nodes/base.py           | 20 ++++-----------
 daemon/core/nodes/interface.py      | 29 +++++++++++++++++++++
 daemon/core/nodes/network.py        | 21 +++++----------
 daemon/core/xml/corexml.py          | 29 ++++++++++-----------
 daemon/tests/test_grpc.py           |  4 +--
 daemon/tests/test_gui.py            |  4 +--
 daemon/tests/test_xml.py            | 40 ++++++++++++++---------------
 10 files changed, 120 insertions(+), 113 deletions(-)

diff --git a/daemon/core/api/grpc/grpcutils.py b/daemon/core/api/grpc/grpcutils.py
index 095c4d0c..5213a835 100644
--- a/daemon/core/api/grpc/grpcutils.py
+++ b/daemon/core/api/grpc/grpcutils.py
@@ -319,6 +319,22 @@ def convert_iface(iface_data: InterfaceData) -> core_pb2.Interface:
     )
 
 
+def convert_link_options(options_data: LinkOptions) -> core_pb2.LinkOptions:
+    return core_pb2.LinkOptions(
+        opaque=options_data.opaque,
+        jitter=options_data.jitter,
+        key=options_data.key,
+        mburst=options_data.mburst,
+        mer=options_data.mer,
+        loss=options_data.loss,
+        bandwidth=options_data.bandwidth,
+        burst=options_data.burst,
+        delay=options_data.delay,
+        dup=options_data.dup,
+        unidirectional=options_data.unidirectional,
+    )
+
+
 def convert_link(link_data: LinkData) -> core_pb2.Link:
     """
     Convert link_data into core protobuf link.
@@ -332,19 +348,7 @@ def convert_link(link_data: LinkData) -> core_pb2.Link:
     iface2 = None
     if link_data.iface2 is not None:
         iface2 = convert_iface(link_data.iface2)
-    options = core_pb2.LinkOptions(
-        opaque=link_data.opaque,
-        jitter=link_data.jitter,
-        key=link_data.key,
-        mburst=link_data.mburst,
-        mer=link_data.mer,
-        loss=link_data.loss,
-        bandwidth=link_data.bandwidth,
-        burst=link_data.burst,
-        delay=link_data.delay,
-        dup=link_data.dup,
-        unidirectional=link_data.unidirectional,
-    )
+    options = convert_link_options(link_data.options)
     return core_pb2.Link(
         type=link_data.link_type.value,
         node1_id=link_data.node1_id,
diff --git a/daemon/core/api/tlv/corehandlers.py b/daemon/core/api/tlv/corehandlers.py
index 88906e0c..3a4351f1 100644
--- a/daemon/core/api/tlv/corehandlers.py
+++ b/daemon/core/api/tlv/corehandlers.py
@@ -343,12 +343,13 @@ class CoreHandler(socketserver.BaseRequestHandler):
         :return: nothing
         """
         logging.debug("handling broadcast link: %s", link_data)
+        options_data = link_data.options
         loss = ""
-        if link_data.loss is not None:
-            loss = str(link_data.loss)
+        if options_data.loss is not None:
+            loss = str(options_data.loss)
         dup = ""
-        if link_data.dup is not None:
-            dup = str(link_data.dup)
+        if options_data.dup is not None:
+            dup = str(options_data.dup)
         iface1 = link_data.iface1
         if iface1 is None:
             iface1 = InterfaceData()
@@ -361,20 +362,20 @@ class CoreHandler(socketserver.BaseRequestHandler):
             [
                 (LinkTlvs.N1_NUMBER, link_data.node1_id),
                 (LinkTlvs.N2_NUMBER, link_data.node2_id),
-                (LinkTlvs.DELAY, link_data.delay),
-                (LinkTlvs.BANDWIDTH, link_data.bandwidth),
+                (LinkTlvs.DELAY, options_data.delay),
+                (LinkTlvs.BANDWIDTH, options_data.bandwidth),
                 (LinkTlvs.LOSS, loss),
                 (LinkTlvs.DUP, dup),
-                (LinkTlvs.JITTER, link_data.jitter),
-                (LinkTlvs.MER, link_data.mer),
-                (LinkTlvs.BURST, link_data.burst),
-                (LinkTlvs.MBURST, link_data.mburst),
+                (LinkTlvs.JITTER, options_data.jitter),
+                (LinkTlvs.MER, options_data.mer),
+                (LinkTlvs.BURST, options_data.burst),
+                (LinkTlvs.MBURST, options_data.mburst),
                 (LinkTlvs.TYPE, link_data.link_type.value),
-                (LinkTlvs.GUI_ATTRIBUTES, link_data.gui_attributes),
-                (LinkTlvs.UNIDIRECTIONAL, link_data.unidirectional),
-                (LinkTlvs.EMULATION_ID, link_data.emulation_id),
+                (LinkTlvs.GUI_ATTRIBUTES, options_data.gui_attributes),
+                (LinkTlvs.UNIDIRECTIONAL, options_data.unidirectional),
+                (LinkTlvs.EMULATION_ID, options_data.emulation_id),
                 (LinkTlvs.NETWORK_ID, link_data.network_id),
-                (LinkTlvs.KEY, link_data.key),
+                (LinkTlvs.KEY, options_data.key),
                 (LinkTlvs.IFACE1_NUMBER, iface1.id),
                 (LinkTlvs.IFACE1_IP4, iface1.ip4),
                 (LinkTlvs.IFACE1_IP4_MASK, iface1.ip4_mask),
@@ -387,7 +388,7 @@ class CoreHandler(socketserver.BaseRequestHandler):
                 (LinkTlvs.IFACE2_MAC, iface2.mac),
                 (LinkTlvs.IFACE2_IP6, iface2.ip6),
                 (LinkTlvs.IFACE2_IP6_MASK, iface2.ip6_mask),
-                (LinkTlvs.OPAQUE, link_data.opaque),
+                (LinkTlvs.OPAQUE, options_data.opaque),
             ],
         )
 
diff --git a/daemon/core/emulator/data.py b/daemon/core/emulator/data.py
index c08a70f0..0c263135 100644
--- a/daemon/core/emulator/data.py
+++ b/daemon/core/emulator/data.py
@@ -142,36 +142,27 @@ class LinkOptions:
     burst: int = None
     mburst: int = None
     gui_attributes: str = None
-    unidirectional: bool = None
+    unidirectional: int = None
     emulation_id: int = None
-    network_id: int = None
     key: int = None
     opaque: str = None
 
 
 @dataclass
 class LinkData:
+    """
+    Represents all data associated with a link.
+    """
+
     message_type: MessageFlags = None
+    link_type: LinkTypes = None
     label: str = None
     node1_id: int = None
     node2_id: int = None
-    delay: float = None
-    bandwidth: float = None
-    loss: float = None
-    dup: float = None
-    jitter: float = None
-    mer: float = None
-    burst: float = None
-    mburst: float = None
-    link_type: LinkTypes = None
-    gui_attributes: str = None
-    unidirectional: int = None
-    emulation_id: int = None
-    network_id: int = None
-    key: int = None
     iface1: InterfaceData = None
     iface2: InterfaceData = None
-    opaque: str = None
+    options: LinkOptions = LinkOptions()
+    network_id: int = None
     color: str = None
 
 
diff --git a/daemon/core/nodes/base.py b/daemon/core/nodes/base.py
index 3c754aa2..a6e4f147 100644
--- a/daemon/core/nodes/base.py
+++ b/daemon/core/nodes/base.py
@@ -224,9 +224,7 @@ class NodeBase(abc.ABC):
 
     def all_link_data(self, flags: MessageFlags = MessageFlags.NONE) -> List[LinkData]:
         """
-        Build CORE Link data for this object. There is no default
-        method for PyCoreObjs as PyCoreNodes do not implement this but
-        PyCoreNets do.
+        Build link data for this node.
 
         :param flags: message flags
         :return: list of link data
@@ -1108,35 +1106,27 @@ class CoreNetworkBase(NodeBase):
                     iface2.ip6 = ip
                     iface2.ip6_mask = mask
 
+            options_data = iface.get_link_options(unidirectional)
             link_data = LinkData(
                 message_type=flags,
                 node1_id=self.id,
                 node2_id=linked_node.id,
                 link_type=self.linktype,
-                unidirectional=unidirectional,
                 iface2=iface2,
-                delay=iface.getparam("delay"),
-                bandwidth=iface.getparam("bw"),
-                dup=iface.getparam("duplicate"),
-                jitter=iface.getparam("jitter"),
-                loss=iface.getparam("loss"),
+                options=options_data,
             )
             all_links.append(link_data)
 
             if not uni:
                 continue
             iface.swapparams("_params_up")
+            options_data = iface.get_link_options(unidirectional)
             link_data = LinkData(
                 message_type=MessageFlags.NONE,
                 node1_id=linked_node.id,
                 node2_id=self.id,
                 link_type=self.linktype,
-                unidirectional=1,
-                delay=iface.getparam("delay"),
-                bandwidth=iface.getparam("bw"),
-                dup=iface.getparam("duplicate"),
-                jitter=iface.getparam("jitter"),
-                loss=iface.getparam("loss"),
+                options=options_data,
             )
             iface.swapparams("_params_up")
             all_links.append(link_data)
diff --git a/daemon/core/nodes/interface.py b/daemon/core/nodes/interface.py
index dc16517f..1fb8b894 100644
--- a/daemon/core/nodes/interface.py
+++ b/daemon/core/nodes/interface.py
@@ -7,6 +7,7 @@ import time
 from typing import TYPE_CHECKING, Callable, Dict, List, Optional, Tuple
 
 from core import utils
+from core.emulator.data import LinkOptions
 from core.emulator.enumerations import MessageFlags, TransportType
 from core.errors import CoreCommandError
 from core.nodes.netclient import LinuxNetClient, get_net_client
@@ -169,6 +170,34 @@ class CoreInterface:
         """
         return self._params.get(key)
 
+    def get_link_options(self, unidirectional: int) -> LinkOptions:
+        """
+        Get currently set params as link options.
+
+        :param unidirectional: unidirectional setting
+        :return: link options
+        """
+        delay = self.getparam("delay")
+        if delay is not None:
+            delay = int(delay)
+        bandwidth = self.getparam("bw")
+        if bandwidth is not None:
+            bandwidth = int(bandwidth)
+        dup = self.getparam("duplicate")
+        if dup is not None:
+            dup = int(dup)
+        jitter = self.getparam("jitter")
+        if jitter is not None:
+            jitter = int(jitter)
+        return LinkOptions(
+            delay=delay,
+            bandwidth=bandwidth,
+            dup=dup,
+            jitter=jitter,
+            loss=self.getparam("loss"),
+            unidirectional=unidirectional,
+        )
+
     def getparams(self) -> List[Tuple[str, float]]:
         """
         Return (key, value) pairs for parameters.
diff --git a/daemon/core/nodes/network.py b/daemon/core/nodes/network.py
index b2f6bbf3..972d54f9 100644
--- a/daemon/core/nodes/network.py
+++ b/daemon/core/nodes/network.py
@@ -884,11 +884,12 @@ class PtpNet(CoreNetwork):
         :return: list of link data
         """
         all_links = []
-
         if len(self.ifaces) != 2:
             return all_links
 
-        iface1, iface2 = self.get_ifaces()
+        ifaces = self.get_ifaces()
+        iface1 = ifaces[0]
+        iface2 = ifaces[1]
         unidirectional = 0
         if iface1.getparams() != iface2.getparams():
             unidirectional = 1
@@ -919,19 +920,15 @@ class PtpNet(CoreNetwork):
                 iface2.ip6 = ip
                 iface2.ip6_mask = mask
 
+        options_data = iface1.get_link_options(unidirectional)
         link_data = LinkData(
             message_type=flags,
             node1_id=iface1.node.id,
             node2_id=iface2.node.id,
             link_type=self.linktype,
-            unidirectional=unidirectional,
-            delay=iface1.getparam("delay"),
-            bandwidth=iface1.getparam("bw"),
-            loss=iface1.getparam("loss"),
-            dup=iface1.getparam("duplicate"),
-            jitter=iface1.getparam("jitter"),
             iface1=iface1_data,
             iface2=iface2_data,
+            options=options_data,
         )
         all_links.append(link_data)
 
@@ -940,19 +937,15 @@ class PtpNet(CoreNetwork):
         if unidirectional:
             iface1_data = InterfaceData(id=iface2.node.get_iface_id(iface2))
             iface2_data = InterfaceData(id=iface1.node.get_iface_id(iface1))
+            options_data = iface2.get_link_options(unidirectional)
             link_data = LinkData(
                 message_type=MessageFlags.NONE,
                 link_type=self.linktype,
                 node1_id=iface2.node.id,
                 node2_id=iface1.node.id,
-                delay=iface2.getparam("delay"),
-                bandwidth=iface2.getparam("bw"),
-                loss=iface2.getparam("loss"),
-                dup=iface2.getparam("duplicate"),
-                jitter=iface2.getparam("jitter"),
-                unidirectional=1,
                 iface1=iface1_data,
                 iface2=iface2_data,
+                options=options_data,
             )
             all_links.append(link_data)
         return all_links
diff --git a/daemon/core/xml/corexml.py b/daemon/core/xml/corexml.py
index 1f92502c..4febe71f 100644
--- a/daemon/core/xml/corexml.py
+++ b/daemon/core/xml/corexml.py
@@ -537,22 +537,22 @@ class CoreXmlWriter:
         is_node1_wireless = isinstance(node1, (WlanNode, EmaneNet))
         is_node2_wireless = isinstance(node2, (WlanNode, EmaneNet))
         if not any([is_node1_wireless, is_node2_wireless]):
+            options_data = link_data.options
             options = etree.Element("options")
-            add_attribute(options, "delay", link_data.delay)
-            add_attribute(options, "bandwidth", link_data.bandwidth)
-            add_attribute(options, "loss", link_data.loss)
-            add_attribute(options, "dup", link_data.dup)
-            add_attribute(options, "jitter", link_data.jitter)
-            add_attribute(options, "mer", link_data.mer)
-            add_attribute(options, "burst", link_data.burst)
-            add_attribute(options, "mburst", link_data.mburst)
-            add_attribute(options, "type", link_data.link_type)
-            add_attribute(options, "gui_attributes", link_data.gui_attributes)
-            add_attribute(options, "unidirectional", link_data.unidirectional)
-            add_attribute(options, "emulation_id", link_data.emulation_id)
+            add_attribute(options, "delay", options_data.delay)
+            add_attribute(options, "bandwidth", options_data.bandwidth)
+            add_attribute(options, "loss", options_data.loss)
+            add_attribute(options, "dup", options_data.dup)
+            add_attribute(options, "jitter", options_data.jitter)
+            add_attribute(options, "mer", options_data.mer)
+            add_attribute(options, "burst", options_data.burst)
+            add_attribute(options, "mburst", options_data.mburst)
+            add_attribute(options, "gui_attributes", options_data.gui_attributes)
+            add_attribute(options, "unidirectional", options_data.unidirectional)
+            add_attribute(options, "emulation_id", options_data.emulation_id)
             add_attribute(options, "network_id", link_data.network_id)
-            add_attribute(options, "key", link_data.key)
-            add_attribute(options, "opaque", link_data.opaque)
+            add_attribute(options, "key", options_data.key)
+            add_attribute(options, "opaque", options_data.opaque)
             if options.items():
                 link_element.append(options)
 
@@ -940,7 +940,6 @@ class CoreXmlReader:
                     options.loss = get_float(options_element, "per")
                 options.unidirectional = get_int(options_element, "unidirectional")
                 options.emulation_id = get_int(options_element, "emulation_id")
-                options.network_id = get_int(options_element, "network_id")
                 options.opaque = options_element.get("opaque")
                 options.gui_attributes = options_element.get("gui_attributes")
 
diff --git a/daemon/tests/test_grpc.py b/daemon/tests/test_grpc.py
index b2a1c312..cff7cd85 100644
--- a/daemon/tests/test_grpc.py
+++ b/daemon/tests/test_grpc.py
@@ -590,7 +590,7 @@ class TestGrpc:
         session.add_link(node.id, switch.id, iface)
         options = core_pb2.LinkOptions(bandwidth=30000)
         link = switch.all_link_data()[0]
-        assert options.bandwidth != link.bandwidth
+        assert options.bandwidth != link.options.bandwidth
 
         # then
         with client.context_connect():
@@ -601,7 +601,7 @@ class TestGrpc:
         # then
         assert response.result is True
         link = switch.all_link_data()[0]
-        assert options.bandwidth == link.bandwidth
+        assert options.bandwidth == link.options.bandwidth
 
     def test_delete_link(self, grpc_server: CoreGrpcServer, ip_prefixes: IpPrefixes):
         # given
diff --git a/daemon/tests/test_gui.py b/daemon/tests/test_gui.py
index c413295a..8f01a2bf 100644
--- a/daemon/tests/test_gui.py
+++ b/daemon/tests/test_gui.py
@@ -201,7 +201,7 @@ class TestGui:
         all_links = switch_node.all_link_data()
         assert len(all_links) == 1
         link = all_links[0]
-        assert link.bandwidth is None
+        assert link.options.bandwidth is None
 
         bandwidth = 50000
         message = coreapi.CoreLinkMessage.create(
@@ -219,7 +219,7 @@ class TestGui:
         all_links = switch_node.all_link_data()
         assert len(all_links) == 1
         link = all_links[0]
-        assert link.bandwidth == bandwidth
+        assert link.options.bandwidth == bandwidth
 
     def test_link_delete_node_to_node(self, coretlv: CoreHandler):
         node1_id = 1
diff --git a/daemon/tests/test_xml.py b/daemon/tests/test_xml.py
index d81fe471..91b598f3 100644
--- a/daemon/tests/test_xml.py
+++ b/daemon/tests/test_xml.py
@@ -347,11 +347,11 @@ class TestXml:
             node = session.nodes[node_id]
             links += node.all_link_data()
         link = links[0]
-        assert options.loss == link.loss
-        assert options.bandwidth == link.bandwidth
-        assert options.jitter == link.jitter
-        assert options.delay == link.delay
-        assert options.dup == link.dup
+        assert options.loss == link.options.loss
+        assert options.bandwidth == link.options.bandwidth
+        assert options.jitter == link.options.jitter
+        assert options.delay == link.options.delay
+        assert options.dup == link.options.dup
 
     def test_link_options_ptp(
         self, session: Session, tmpdir: TemporaryFile, ip_prefixes: IpPrefixes
@@ -414,11 +414,11 @@ class TestXml:
             node = session.nodes[node_id]
             links += node.all_link_data()
         link = links[0]
-        assert options.loss == link.loss
-        assert options.bandwidth == link.bandwidth
-        assert options.jitter == link.jitter
-        assert options.delay == link.delay
-        assert options.dup == link.dup
+        assert options.loss == link.options.loss
+        assert options.bandwidth == link.options.bandwidth
+        assert options.jitter == link.options.jitter
+        assert options.delay == link.options.delay
+        assert options.dup == link.options.dup
 
     def test_link_options_bidirectional(
         self, session: Session, tmpdir: TemporaryFile, ip_prefixes: IpPrefixes
@@ -494,13 +494,13 @@ class TestXml:
         assert len(links) == 2
         link1 = links[0]
         link2 = links[1]
-        assert options1.bandwidth == link1.bandwidth
-        assert options1.delay == link1.delay
-        assert options1.loss == link1.loss
-        assert options1.dup == link1.dup
-        assert options1.jitter == link1.jitter
-        assert options2.bandwidth == link2.bandwidth
-        assert options2.delay == link2.delay
-        assert options2.loss == link2.loss
-        assert options2.dup == link2.dup
-        assert options2.jitter == link2.jitter
+        assert options1.bandwidth == link1.options.bandwidth
+        assert options1.delay == link1.options.delay
+        assert options1.loss == link1.options.loss
+        assert options1.dup == link1.options.dup
+        assert options1.jitter == link1.options.jitter
+        assert options2.bandwidth == link2.options.bandwidth
+        assert options2.delay == link2.options.delay
+        assert options2.loss == link2.options.loss
+        assert options2.dup == link2.options.dup
+        assert options2.jitter == link2.options.jitter