From e0a5f4d7460e6cde9e7b3ddf4262fea7df6b27ab Mon Sep 17 00:00:00 2001
From: Ross McFarland <rwmcfa1@gmail.com>
Date: Tue, 8 Feb 2022 09:31:42 -0800
Subject: [PATCH] Rework _AggregateTarget to dynamically handle SUPPORTS

---
 CHANGELOG.md                  |  2 ++
 octodns/manager.py            | 27 ++++++++++++++-------------
 tests/helpers.py              |  2 ++
 tests/test_octodns_manager.py | 14 ++++++++++++++
 4 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0d8ebcf..99b40d0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -48,6 +48,8 @@
 
 * Additional FQDN validation to ALIAS/CNAME value, MX exchange, SRV target and
   tests of the functionality.
+* _AggregateTarget has more complete handling of SUPPORTS* functionality,
+  mostly applicable for the compare operation.
 
 ## v0.9.14 - 2021-10-10 - A new supports system
 
diff --git a/octodns/manager.py b/octodns/manager.py
index f8e9925..c8a4263 100644
--- a/octodns/manager.py
+++ b/octodns/manager.py
@@ -24,6 +24,9 @@ class _AggregateTarget(object):
 
     def __init__(self, targets):
         self.targets = targets
+        self.SUPPORTS = targets[0].SUPPORTS
+        for target in targets[1:]:
+            self.SUPPORTS = self.SUPPORTS & target.SUPPORTS
 
     def supports(self, record):
         for target in self.targets:
@@ -31,19 +34,17 @@ class _AggregateTarget(object):
                 return False
         return True
 
-    @property
-    def SUPPORTS_GEO(self):
-        for target in self.targets:
-            if not target.SUPPORTS_GEO:
-                return False
-        return True
-
-    @property
-    def SUPPORTS_DYNAMIC(self):
-        for target in self.targets:
-            if not target.SUPPORTS_DYNAMIC:
-                return False
-        return True
+    def __getattr__(self, name):
+        if name.startswith('SUPPORTS_'):
+            # special case to handle any current or future SUPPORTS_* by
+            # returning whether all providers support the requested
+            # functionality.
+            for target in self.targets:
+                if not getattr(target, name):
+                    return False
+            return True
+        klass = self.__class__.__name__
+        raise AttributeError(f'{klass} object has no attribute {name}')
 
 
 class MakeThreadFuture(object):
diff --git a/tests/helpers.py b/tests/helpers.py
index 17b0115..0d0f54d 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -41,6 +41,7 @@ class SimpleProvider(object):
 class GeoProvider(object):
     SUPPORTS_GEO = True
     SUPPORTS_DYNAMIC = False
+    SUPPORTS = set(('A', 'AAAA', 'TXT'))
     id = 'test'
 
     def __init__(self, id='test'):
@@ -59,6 +60,7 @@ class GeoProvider(object):
 class DynamicProvider(object):
     SUPPORTS_GEO = False
     SUPPORTS_DYNAMIC = True
+    SUPPORTS = set(('A', 'AAAA', 'TXT'))
     id = 'test'
 
     def __init__(self, id='test'):
diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py
index 06ec7ca..d63e84d 100644
--- a/tests/test_octodns_manager.py
+++ b/tests/test_octodns_manager.py
@@ -235,6 +235,20 @@ class TestManager(TestCase):
         dynamic = DynamicProvider()
         nosshfp = NoSshFpProvider()
 
+        targets = [simple, geo]
+        at = _AggregateTarget(targets)
+        # expected targets
+        self.assertEqual(targets, at.targets)
+        # union of their SUPPORTS
+        self.assertEqual(set(('A')), at.SUPPORTS)
+
+        # unknown property will go up into super and throw the normal
+        # exception
+        with self.assertRaises(AttributeError) as ctx:
+            at.FOO
+        self.assertEqual('_AggregateTarget object has no attribute FOO',
+                         str(ctx.exception))
+
         self.assertFalse(_AggregateTarget([simple, simple]).SUPPORTS_GEO)
         self.assertFalse(_AggregateTarget([simple, geo]).SUPPORTS_GEO)
         self.assertFalse(_AggregateTarget([geo, simple]).SUPPORTS_GEO)