From 4a6fae34b3fdf0551207d46dd12b53fb97c51580 Mon Sep 17 00:00:00 2001
From: Ross McFarland <rwmcfa1@gmail.com>
Date: Sat, 1 Oct 2022 19:08:24 -0700
Subject: [PATCH] PtrValue should no longer be using _TargetValue now that it
 acceps multiple values

---
 octodns/record/__init__.py   | 39 +++++++++++++++++++++-----------
 tests/test_octodns_record.py | 43 +++++++++++++++++++++++++++++++-----
 2 files changed, 64 insertions(+), 18 deletions(-)

diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py
index 31ed400..e4efcc8 100644
--- a/octodns/record/__init__.py
+++ b/octodns/record/__init__.py
@@ -1778,26 +1778,39 @@ class NsRecord(ValuesMixin, Record):
 Record.register_type(NsRecord)
 
 
-class PtrValue(_TargetValue):
+class PtrValue(str):
     @classmethod
-    def validate(cls, values, _type):
-        if not isinstance(values, list):
-            values = [values]
+    def parse_rdata_text(self, value):
+        return value
 
+    @classmethod
+    def validate(cls, data, _type):
+        if not data:
+            return ['missing value(s)']
+        elif not isinstance(data, (list, tuple)):
+            data = (data,)
         reasons = []
-
-        if not values:
-            reasons.append('missing values')
-
-        for value in values:
-            reasons.extend(super().validate(value, _type))
-
+        for value in data:
+            value = idna_encode(value)
+            if not FQDN(value, allow_underscores=True).is_valid:
+                reasons.append(
+                    f'Invalid PTR value "{value}" is not a valid FQDN.'
+                )
+            elif not value.endswith('.'):
+                reasons.append(f'PTR value "{value}" missing trailing .')
         return reasons
 
     @classmethod
     def process(cls, values):
-        supr = super()
-        return [supr.process(v) for v in values]
+        return [cls(v) for v in values]
+
+    def __new__(cls, v):
+        v = idna_encode(v)
+        return super().__new__(cls, v)
+
+    @property
+    def rdata_text(self):
+        return self
 
 
 class PtrRecord(ValuesMixin, Record):
diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py
index fb9f726..43db74a 100644
--- a/tests/test_octodns_record.py
+++ b/tests/test_octodns_record.py
@@ -25,6 +25,7 @@ from octodns.record import (
     NaptrValue,
     NsRecord,
     PtrRecord,
+    PtrValue,
     Record,
     RecordException,
     Rr,
@@ -217,12 +218,12 @@ class TestRecord(TestCase):
         upper_record = PtrRecord(
             self.zone,
             'PtrUppwerValue',
-            {'ttl': 30, 'type': 'PTR', 'value': 'GITHUB.COM'},
+            {'ttl': 30, 'type': 'PTR', 'value': 'GITHUB.COM.'},
         )
         lower_record = PtrRecord(
             self.zone,
             'PtrLowerValue',
-            {'ttl': 30, 'type': 'PTR', 'value': 'github.com'},
+            {'ttl': 30, 'type': 'PTR', 'value': 'github.com.'},
         )
         self.assertEqual(upper_record.value, lower_record.value)
 
@@ -3871,7 +3872,7 @@ class TestRecordValidation(TestCase):
             ctx.exception.reasons,
         )
 
-    def test_PTR(self):
+    def test_ptr(self):
         # doesn't blow up (name & zone here don't make any sense, but not
         # important)
         Record.new(
@@ -3881,7 +3882,12 @@ class TestRecordValidation(TestCase):
         # missing value
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {'type': 'PTR', 'ttl': 600})
-        self.assertEqual(['missing values'], ctx.exception.reasons)
+        self.assertEqual(['missing value(s)'], ctx.exception.reasons)
+
+        # empty value
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {'type': 'PTR', 'ttl': 600, 'value': ''})
+        self.assertEqual(['missing value(s)'], ctx.exception.reasons)
 
         # not a valid FQDN
         with self.assertRaises(ValidationError) as ctx:
@@ -3889,7 +3895,8 @@ class TestRecordValidation(TestCase):
                 self.zone, '', {'type': 'PTR', 'ttl': 600, 'value': '_.'}
             )
         self.assertEqual(
-            ['PTR value "_." is not a valid FQDN'], ctx.exception.reasons
+            ['Invalid PTR value "_." is not a valid FQDN.'],
+            ctx.exception.reasons,
         )
 
         # no trailing .
@@ -3901,6 +3908,32 @@ class TestRecordValidation(TestCase):
             ['PTR value "foo.bar" missing trailing .'], ctx.exception.reasons
         )
 
+    def test_ptr_rdata_text(self):
+
+        # anything goes, we're a noop
+        for s in (
+            None,
+            '',
+            'word',
+            42,
+            42.43,
+            '1.2.3',
+            'some.words.that.here',
+            '1.2.word.4',
+            '1.2.3.4',
+        ):
+            self.assertEqual(s, PtrValue.parse_rdata_text(s))
+
+        zone = Zone('unit.tests.', [])
+        a = PtrRecord(zone, 'a', {'ttl': 42, 'value': 'some.target.'})
+        self.assertEqual('some.target.', a.values[0].rdata_text)
+
+        a = PtrRecord(
+            zone, 'a', {'ttl': 42, 'values': ['some.target.', 'second.target.']}
+        )
+        self.assertEqual('second.target.', a.values[0].rdata_text)
+        self.assertEqual('some.target.', a.values[1].rdata_text)
+
     def test_SSHFP(self):
         # doesn't blow up
         Record.new(