rewrite property function to better handle caching of zones

octodns/provider/route53.py

@@ -643,40 +643,49 @@ class Route53Provider(BaseProvider):
         self._r53_rrsets = {}
         self._health_checks = None
 
-    @property
-    def r53_zones(self):
+    def _get_zone_id_by_name(self, name):
+        # attempt to get zone by name
+        # limited to one as this should be unique
+        id = None
+        resp = self._conn.list_hosted_zones_by_name(
+            DNSName=name, MaxItems="1"
+        )
+        if len(resp['HostedZones']) != 0:
+            # if there is a response that starts with the name
+            if resp['HostedZones'][0]['Name'].startswith(name):
+                id = resp['HostedZones'][0]['Id']
+                self.log.debug('get_zones_by_name:   id=%s', id)
+        return id
+
+    def update_r53_zones(self, name):
         if self._r53_zones is None:
-            self.log.debug('r53_zones: loading')
-            zones = {}
-            more = True
-            start = {}
-            while more:
-                resp = self._conn.list_hosted_zones(**start)
-                for z in resp['HostedZones']:
-                    zones[z['Name']] = z['Id']
-                more = resp['IsTruncated']
-                start['Marker'] = resp.get('NextMarker', None)
-
-            self._r53_zones = zones
-
-        return self._r53_zones
+            if self.get_zones_by_name:
+                id = self._get_zone_id_by_name(name)
+                zones = {}
+                zones[name] = id
+                self._r53_zones = zones
+            else:
+                self.log.debug('r53_zones: loading')
+                zones = {}
+                more = True
+                start = {}
+                while more:
+                    resp = self._conn.list_hosted_zones(**start)
+                    for z in resp['HostedZones']:
+                        zones[z['Name']] = z['Id']
+                    more = resp['IsTruncated']
+                    start['Marker'] = resp.get('NextMarker', None)
+                self._r53_zones = zones
+        else:
+            if name not in self._r53_zones and self.get_zones_by_name:
+                id = self._get_zone_id_by_name(name)
+                self._r53_zones[name] = id
 
     def _get_zone_id(self, name, create=False):
         self.log.debug('_get_zone_id: name=%s', name)
-        if self.get_zones_by_name:
-            # attempt to get zone by name
-            # limited to one as this should be unique
-            response = self._conn.list_hosted_zones_by_name(
-                DNSName=name, MaxItems="1"
-            )
-            if len(response['HostedZones']) != 0:
-                # if there is a response that starts with the name
-                if response['HostedZones'][0]['Name'].startswith(name):
-                    id = response['HostedZones'][0]['Id']
-                    self.log.debug('get_zones_by_name:   id=%s', id)
-                    return id
-        elif name in self.r53_zones:
-            id = self.r53_zones[name]
+        self.update_r53_zones(name)
+        if name in self._r53_zones:
+            id = self._r53_zones[name]
             self.log.debug('_get_zone_id:   id=%s', id)
             return id
         if create:
@@ -691,7 +700,7 @@ class Route53Provider(BaseProvider):
             else:
                 resp = self._conn.create_hosted_zone(Name=name,
                                                      CallerReference=ref)
-            self.r53_zones[name] = id = resp['HostedZone']['Id']
+            self._r53_zones[name] = id = resp['HostedZone']['Id']
             return id
         return None
 

tests/test_octodns_provider_route53.py

@@ -1753,7 +1753,7 @@ class TestRoute53Provider(TestCase):
         stubber = Stubber(provider._conn)
         stubber.activate()
 
-        list_hosted_zones_by_name_resp = {
+        list_hosted_zones_by_name_resp_1 = {
             'HostedZones': [{
                 'Id': 'z42',
                 'Name': 'unit.tests.',
@@ -1770,9 +1770,26 @@ class TestRoute53Provider(TestCase):
             'MaxItems': 'string'
         }
 
+        list_hosted_zones_by_name_resp_2 = {
+            'HostedZones': [{
+                'Id': 'z43',
+                'Name': 'unit2.tests.',
+                'CallerReference': 'abc',
+                'Config': {
+                    'Comment': 'string',
+                    'PrivateZone': False
+                },
+                'ResourceRecordSetCount': 123,
+            }, ],
+            'DNSName': 'unit2.tests.',
+            'HostedZoneId': 'z43',
+            'IsTruncated': False,
+            'MaxItems': 'string'
+        }
+
         stubber.add_response(
             'list_hosted_zones_by_name',
-            list_hosted_zones_by_name_resp,
+            list_hosted_zones_by_name_resp_1,
             {'DNSName': 'unit.tests.', 'MaxItems': '1'}
         )
 
@@ -1782,6 +1799,18 @@ class TestRoute53Provider(TestCase):
         self.assertEquals([], extra)
         stubber.assert_no_pending_responses()
 
+        stubber.add_response(
+            'list_hosted_zones_by_name',
+            list_hosted_zones_by_name_resp_2,
+            {'DNSName': 'unit2.tests.', 'MaxItems': '1'}
+        )
+
+        # empty is empty
+        desired = Zone('unit2.tests.', [])
+        extra = provider._extra_changes(desired=desired, changes=[])
+        self.assertEquals([], extra)
+        stubber.assert_no_pending_responses()
+
     def test_zone_not_found_get_zones_by_name(self):
         provider = Route53Provider(
             'test', 'abc', '123', get_zones_by_name=True)
@@ -1837,18 +1866,6 @@ class TestRoute53Provider(TestCase):
             'MaxItems': 'string'
         }
 
-        # list_hosted_zones_by_name gets called 3 times in this process
-        # so adding 3 responses
-        stubber.add_response(
-            'list_hosted_zones_by_name',
-            list_hosted_zones_by_name_resp,
-            {'DNSName': 'unit.tests.', 'MaxItems': '1'}
-        )
-        stubber.add_response(
-            'list_hosted_zones_by_name',
-            list_hosted_zones_by_name_resp,
-            {'DNSName': 'unit.tests.', 'MaxItems': '1'}
-        )
         stubber.add_response(
             'list_hosted_zones_by_name',
             list_hosted_zones_by_name_resp,