Cleanup of visitors and visitor composition bad architecture

cosmo/netbox_autodescribable_mixin.py

@@ -5,7 +5,7 @@
 from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
-    from .autodesc import AbstractComposableAutoDescription
+    from cosmo.autodesc import AbstractComposableAutoDescription
 
 
 class AutoDescribableMixin(MutableMapping, metaclass=ABCMeta):

cosmo/serializer.py

@@ -3,7 +3,7 @@
 
 from deepmerge import Merger
 
-from cosmo.autodescvisitor import MutatingAutoDescVisitor
+from cosmo.visitors.autodesc import MutatingAutoDescVisitor
 from cosmo.common import (
     deepsort,
     DeviceSerializationError,
@@ -13,16 +13,25 @@
 )
 from cosmo.features import features
 from cosmo.log import error
-from cosmo.netbox_types import DeviceType, CosmoLoopbackType, AbstractNetboxType
-from cosmo.loopbacks import LoopbackHelper
+from cosmo.netbox_types import AbstractNetboxType
+from cosmo.visitors.helpers.loopbacks import LoopbackHelper
 from cosmo.netbox_types import DeviceType, CosmoLoopbackType
-from cosmo.switchvisitor import SwitchDeviceExporterVisitor
-from cosmo.routervisitor import RouterDeviceExporterVisitor
+from cosmo.visitors.router_bgpcpe import RouterBgpCpeExporterVisitor
+from cosmo.visitors.router_l2vpn import (
+    RouterL2VPNValidatorVisitor,
+    RouterL2VPNExporterVisitor,
+)
+from cosmo.visitors.switch import SwitchDeviceExporterVisitor
+from cosmo.visitors.router import RouterDeviceExporterVisitor
+
+# serializer function type
+S = Callable[[CosmoOutputType, AbstractNetboxType], None]
 
 
 class AbstractSerializer(metaclass=ABCMeta):
     def __init__(self, device):
         self.device = DeviceType(device)
+        self.serializers: list[S] = []
 
     @staticmethod
     def getMerger():
@@ -39,6 +48,16 @@ def getMerger():
         )
         return merger
 
+    def exportTemplateMethod(
+        self, accept: Callable[[AbstractNetboxType], CosmoOutputType]
+    ) -> S:
+        def export(device_stub: CosmoOutputType, value: AbstractNetboxType):
+            new = accept(value)
+            if new:
+                device_stub = self.getMerger().merge(device_stub, new)
+
+        return export
+
     @staticmethod
     def autoDescPreprocess(_: CosmoOutputType, value: AbstractNetboxType):
         if not features.featureIsEnabled("interface-auto-descriptions"):
@@ -67,6 +86,14 @@ def processErrors(self, latest_errors):
                 str(first_error), on=self.device
             ) from first_error
 
+    def serialize(self) -> CosmoOutputType | Never:
+        device_stub: CosmoOutputType = {}
+        latest_errors: list[AbstractRecoverableError] = []
+        for s in self.serializers:
+            latest_errors.extend(self.walk(device_stub, s))
+        self.processErrors(latest_errors)
+        return deepsort(device_stub)
+
 
 class RouterSerializer(AbstractSerializer):
     def __init__(self, device, l2vpn_list, loopbacks, cosmo_config):
@@ -87,44 +114,50 @@ def __init__(self, device, l2vpn_list, loopbacks, cosmo_config):
             for (key, loopback) in self.loopbacks.items()
         }
         loopback_helper = LoopbackHelper(loopbacks)
+        # main visitor
         self.router_device_export_visitor = RouterDeviceExporterVisitor(
             loopbacks=loopback_helper, cosmo_config=cosmo_config
         )
         if self.allow_private_ips:
             self.router_device_export_visitor.allowPrivateIPs()
+        # supplementary chainable visitors
+        self.router_bgpcpe_export_visitor = RouterBgpCpeExporterVisitor(
+            cosmo_config=cosmo_config,
+        )
+        self.l2vpn_validator = RouterL2VPNValidatorVisitor(
+            cosmo_config=cosmo_config,
+            loopbacks=loopback_helper,
+        )
+        self.l2vpn_exporter = RouterL2VPNExporterVisitor(
+            cosmo_config=cosmo_config,
+            loopbacks=loopback_helper,
+        )
+
+        self.serializers.extend(
+            [
+                self.autoDescPreprocess,
+                self.exportTemplateMethod(self.l2vpn_validator.accept),
+                self.exportTemplateMethod(self.router_device_export_visitor.accept),
+                self.exportTemplateMethod(self.l2vpn_exporter.accept),
+                self.exportTemplateMethod(self.router_bgpcpe_export_visitor.accept),
+            ]
+        )
 
     def allowPrivateIPs(self):
         self.router_device_export_visitor.allowPrivateIPs()
         return self
 
-    def routerExport(self, device_stub: CosmoOutputType, value: AbstractNetboxType):
-        new = self.router_device_export_visitor.accept(value)
-        if new:
-            device_stub = self.getMerger().merge(device_stub, new)
-
-    def serialize(self) -> CosmoOutputType | Never:
-        device_stub: CosmoOutputType = {}
-        latest_errors: list[AbstractRecoverableError] = []
-        latest_errors.extend(self.walk(device_stub, self.autoDescPreprocess))
-        latest_errors.extend(self.walk(device_stub, self.routerExport))
-        self.processErrors(latest_errors)
-        return deepsort(device_stub)
-
 
 class SwitchSerializer(AbstractSerializer):
     def __init__(self, device, cosmo_config):
         super().__init__(device)
         self._cosmo_config = cosmo_config
-
-    def switchExport(self, device_stub: CosmoOutputType, value: AbstractNetboxType):
-        new = SwitchDeviceExporterVisitor(cosmo_config=self._cosmo_config).accept(value)
-        if new:
-            device_stub = self.getMerger().merge(device_stub, new)
-
-    def serialize(self) -> CosmoOutputType | Never:
-        device_stub: CosmoOutputType = {}
-        latest_errors: list[AbstractRecoverableError] = []
-        latest_errors.extend(self.walk(device_stub, self.autoDescPreprocess))
-        latest_errors.extend(self.walk(device_stub, self.switchExport))
-        self.processErrors(latest_errors)
-        return deepsort(device_stub)
+        self.switch_device_export_visitor = SwitchDeviceExporterVisitor(
+            cosmo_config=cosmo_config
+        )
+        self.serializers.extend(
+            [
+                self.autoDescPreprocess,
+                self.exportTemplateMethod(self.switch_device_export_visitor.accept),
+            ]
+        )

cosmo/abstractroutervisitor.py → cosmo/visitors/abc.py

@@ -1,7 +1,13 @@
+import abc
 from abc import ABC
 
-from cosmo.netbox_types import CosmoLoopbackType
-from cosmo.visitors import AbstractNoopNetboxTypesVisitor
+
+class AbstractNoopNetboxTypesVisitor(abc.ABC):
+    def accept(self, o):
+        # use raise NotImplementedError(f"unsupported type {o}")
+        # when you're adding new types and you want to check your
+        # visitor gets everything
+        return
 
 
 class AbstractRouterExporterVisitor(AbstractNoopNetboxTypesVisitor, ABC):

cosmo/autodescvisitor.py → cosmo/visitors/autodesc.py

@@ -8,7 +8,7 @@
 )
 from cosmo.common import head, strictly_decreasing, AutoDescriptionError
 from cosmo.netbox_types import InterfaceType
-from cosmo.visitors import AbstractNoopNetboxTypesVisitor
+from cosmo.visitors.abc import AbstractNoopNetboxTypesVisitor
 
 
 class MutatingAutoDescVisitor(AbstractNoopNetboxTypesVisitor):

cosmo/l2vpnhelpertypes.py → cosmo/visitors/l2vpnhelpertypes.py

@@ -3,25 +3,23 @@
 from multimethod import multimethod as singledispatchmethod
 from typing import NoReturn
 
-from cosmo.abstractroutervisitor import AbstractRouterExporterVisitor
+from cosmo.visitors.abc import AbstractRouterExporterVisitor
 from cosmo.common import (
     head,
     CosmoOutputType,
     L2VPNSerializationError,
-    DeviceSerializationError,
 )
 from cosmo.config.cosmo_config import CosmoConfig
-from cosmo.loopbacks import LoopbackHelper
+from cosmo.visitors.helpers.loopbacks import LoopbackHelper
 from cosmo.manufacturers import ManufacturerFactoryFromDevice
-from cosmo.vrfhelper import TVRFHelpers
+from cosmo.visitors.helpers.vrf import TVRFHelpers
 from cosmo.log import warn
 from cosmo.netbox_types import (
     InterfaceType,
     VLANType,
     AbstractNetboxType,
     DeviceType,
     L2VPNType,
-    CosmoLoopbackType,
     L2VPNTerminationType,
 )
 

cosmo/routervisitor.py → cosmo/visitors/router.py

@@ -8,20 +8,18 @@
 from cosmo.config.cosmo_config import CosmoConfig
 from cosmo.autodesc import AbstractComposableAutoDescription
 from cosmo.log import warn
-from cosmo.abstractroutervisitor import AbstractRouterExporterVisitor
+from cosmo.visitors.abc import AbstractRouterExporterVisitor
 from cosmo.common import (
     InterfaceSerializationError,
     head,
     StaticRouteSerializationError,
     APP_NAME,
-    DeviceSerializationError,
 )
-from cosmo.loopbacks import LoopbackHelper
-from cosmo.vrfhelper import TVRFHelpers
+from cosmo.visitors.helpers.loopbacks import LoopbackHelper
+from cosmo.visitors.helpers.vrf import TVRFHelpers
 from cosmo.manufacturers import ManufacturerFactoryFromDevice, AbstractManufacturer
-from cosmo.routerbgpcpevisitor import RouterBgpCpeExporterVisitor
 from cosmo.features import features
-from cosmo.routerl2vpnvisitor import (
+from cosmo.visitors.router_l2vpn import (
     RouterL2VPNValidatorVisitor,
     RouterL2VPNExporterVisitor,
 )
@@ -46,16 +44,6 @@ class RouterDeviceExporterVisitor(AbstractRouterExporterVisitor, TVRFHelpers):
     def __init__(self, loopbacks: LoopbackHelper, cosmo_config: CosmoConfig):
         self._cosmo_config = cosmo_config
         self.asn = self._cosmo_config["asn"]
-        # Note: I have to use composition since singledispatchmethod does not work well with inheritance
-        self.l2vpn_exporter = RouterL2VPNExporterVisitor(
-            loopbacks=loopbacks, cosmo_config=self._cosmo_config
-        )
-        self.l2vpn_validator = RouterL2VPNValidatorVisitor(
-            loopbacks=loopbacks, cosmo_config=self._cosmo_config
-        )
-        self.bgpcpe_exporter = RouterBgpCpeExporterVisitor(
-            cosmo_config=self._cosmo_config
-        )
         self.loopbacks = loopbacks
         self.allow_private_ips = features.featureIsEnabled(
             "allow-private-ips-default-vrf"
@@ -76,10 +64,6 @@ def disallowPrivateIPs(self):
     def accept(self, o):
         return super().accept(o)
 
-    @accept.register
-    def _(self, o: L2VPNType):
-        return self.l2vpn_validator.accept(o)
-
     @accept.register
     def _(self, o: DeviceType):
         if not o.isCompositeRoot():  # not root, do not process!
@@ -346,7 +330,7 @@ def _(self, o: InterfaceType):
             # guard: do not process VLAN interface info
             return
         if o.hasParentAboveWithType(L2VPNTerminationType):
-            return self.l2vpn_exporter.accept(o)
+            return None  # guard: will be processed by L2VPN exporter
         # interface in interface is lag info
         if o.hasParentAboveWithType(InterfaceType):
             if o.isUnderKeyNameForParentAboveWithType("lag", InterfaceType):
@@ -515,7 +499,7 @@ def processUntaggedVLAN(self, o: VLANType):
     @accept.register
     def _(self, o: VLANType):
         if o.hasParentAboveWithType(L2VPNTerminationType):
-            return self.l2vpn_exporter.accept(o)
+            return None  # will be processed by L2VPN Exporter
         parent_interface = o.getParent(InterfaceType)
         if (
             parent_interface
@@ -849,8 +833,3 @@ def _(self, o: TagType):
                     warn(f"{APP_NAME} doesn't know this bgp tag.", o)
             case _:
                 warn(f"{APP_NAME} doesn't know this tag.", o)
-
-    @accept.register
-    def _(self, o: List[TagType]):
-        if "bgp:cpe" in o:
-            return self.bgpcpe_exporter.accept(o)