Fixing issue with batch change serialization (#51)

Fixes #51  There was an issue with how we serialized:

1. we were looking for a field named `recordData`, when it was just `record`
2. during deserialization, we were not converting the `record` properly using the `rdata_converter`
3. Also fixed #50, as it was a small serialization issue as well

.gitignore

@@ -107,4 +107,5 @@ venv.bak/
 # misc
 .idea/
 .DS_Store
+.vscode
 

func_tests/test_client.py

@@ -1,5 +1,8 @@
 from func_tests.vinyldns_context import vinyldns_test_context
-from vinyldns.record import RecordSet, RecordType, AData
+from vinyldns.batch_change import AddRecordChange, DeleteRecordSetChange, BatchChange, BatchChangeRequest, \
+    DeleteRecordSet, AddRecord, BatchChangeSummary, ListBatchChangeSummaries
+from vinyldns.record import RecordSet, RecordType, AData, AAAAData, PTRData
+from func_tests.utils import wait_until_record_set_exists
 
 
 def test_list_zones(vinyldns_test_context):
@@ -36,3 +39,37 @@ def test_record_sets(vinyldns_test_context):
     assert change.record_set.name == rs.name
     assert all([l.__dict__ == r.__dict__ for l, r in zip(change.record_set.records, rs.records)])
 
+    rs = change.record_set
+    wait_until_record_set_exists(vinyldns_test_context.client, rs.zone_id, rs.id)
+
+    r = vinyldns_test_context.client.get_record_set(rs.zone_id, rs.id)
+    assert r.id == rs.id
+    assert r.name == rs.name
+    assert r.ttl == rs.ttl
+    assert all([l.__dict__ == r.__dict__ for l, r in zip(rs.records, r.records)])
+
+
+def test_batch_change(vinyldns_test_context):
+    changes = [
+        AddRecord('change-test.vinyldns.', RecordType.A, 200, AData('192.0.2.111')),
+        AddRecord('192.0.2.111', RecordType.PTR, 200, PTRData('change-test.vinyldns.'))
+    ]
+    r = vinyldns_test_context.client.create_batch_change(
+        BatchChangeRequest(changes, 'comments', vinyldns_test_context.group.id)
+    )
+    assert r.user_id == 'ok'
+    assert r.user_name == 'ok'
+    assert r.comments == 'comments'
+
+    change1 = r.changes[0]
+    assert change1.input_name == 'change-test.vinyldns.'
+    assert change1.ttl == 200
+    assert change1.record.address == '192.0.2.111'
+    assert change1.type == RecordType.A
+
+    change2 = r.changes[1]
+    assert change2.input_name == '192.0.2.111'
+    assert change2.ttl == 200
+    assert change2.record.ptrdname == 'change-test.vinyldns.'
+    assert change2.type == RecordType.PTR
+

func_tests/utils.py

@@ -17,9 +17,6 @@ def wait_until_zone_exists(vinyldns_client, zone_id):
         time.sleep(RETRY_WAIT)
         retries -= 1
 
-    if zone is None:
-        print("Issue on zone create: {}".format(json.dumps(zone)))
-
     assert zone is not None
 
 
@@ -34,7 +31,18 @@ def wait_until_zone_deleted(vinyldns_client, zone_id):
         time.sleep(RETRY_WAIT)
         retries -= 1
 
-    if zone is not None:
-        print("Zone was not deleted: {}".format(json.dumps(zone)))
-
     assert zone is None
+
+
+def wait_until_record_set_exists(vinyldns_client, zone_id, rs_id):
+    """
+    Waits until the zone exists
+    """
+    rs = vinyldns_client.get_record_set(zone_id, rs_id)
+    retries = MAX_RETRIES
+    while (rs is None) and retries > 0:
+        rs = vinyldns_client.get_record_set(zone_id, rs_id)
+        time.sleep(RETRY_WAIT)
+        retries -= 1
+
+    assert rs is not None

func_tests/vinyldns_context.py

@@ -13,8 +13,8 @@ class VinylDNSContext(object):
     """
     def __init__(self):
         self.client = VinylDNSClient("http://localhost:9000", "okAccessKey", "okSecretKey")
-        self.tear_down()
         self.group = None
+        self.tear_down()
 
         group = Group(
             name='vinyldns-python-test-group',
@@ -33,7 +33,15 @@ def __init__(self):
         )
         zone_change = self.client.connect_zone(zone)
         self.zone = zone_change.zone
-        wait_until_zone_exists(self.client, self.zone.id)
+
+        reverse_zone = Zone(
+            name='2.0.192.in-addr.arpa.',
+            email='[email protected]',
+            admin_group_id=self.group.id
+        )
+        zone_change = self.client.connect_zone(reverse_zone)
+        self.reverse_zone = zone_change.zone
+        wait_until_zone_exists(self.client, self.reverse_zone.id)
 
     # finalizer called by py.test when the simulation is torn down
     def tear_down(self):
@@ -48,14 +56,13 @@ def clear_groups(self):
     def clear_zones(self):
         # Get the groups for the ok user
         groups = self.client.list_all_my_groups().groups
-        group_ids = map(lambda x: x.id, groups)
+        group_ids = list(map(lambda x: x.id, groups))
 
         zones = self.client.list_zones().zones
 
         # we only want to delete zones that the ok user "owns"
-        zones_to_delete = filter(lambda x: (x.admin_group_id in group_ids), zones)
-
-        zoneids_to_delete = map(lambda x: x.id, zones_to_delete)
+        zones_to_delete = list(filter(lambda x: x.admin_group_id in group_ids, zones))
+        zoneids_to_delete = list(map(lambda x: x.id, zones_to_delete))
 
         for id in zoneids_to_delete:
             self.client.abandon_zone(id)

src/vinyldns/batch_change.py

@@ -14,6 +14,7 @@
 """TODO: Add module docstring."""
 
 from vinyldns.serdes import parse_datetime, map_option
+from vinyldns.record import rdata_converters
 
 
 class AddRecord(object):
@@ -70,15 +71,15 @@ def from_dict(d):
 
 
 class AddRecordChange(object):
-    def __init__(self, zone_id, zone_name, record_name, input_name, type, ttl, record_data, status, id,
+    def __init__(self, zone_id, zone_name, record_name, input_name, type, ttl, record, status, id,
                  system_message=None, record_change_id=None, record_set_id=None):
         self.zone_id = zone_id
         self.zone_name = zone_name
         self.record_name = record_name
         self.input_name = input_name
         self.type = type
         self.ttl = ttl
-        self.record_data = record_data,
+        self.record = record
         self.status = status
         self.id = id
         self.system_message = system_message
@@ -95,7 +96,7 @@ def from_dict(d):
             input_name=d['inputName'],
             type=d['type'],
             ttl=d['ttl'],
-            record_data=d['recordData'],
+            record=rdata_converters[d['type']](d['record']),
             status=d['status'],
             id=d['id'],
             system_message=d.get('systemMessage'),

src/vinyldns/client.py

@@ -521,7 +521,7 @@ def get_record_set(self, zone_id, rs_id, **kwargs):
         url = urljoin(self.index_url, u'/zones/{0}/recordsets/{1}'.format(zone_id, rs_id))
 
         response, data = self.__make_request(url, u'GET', self.headers, None, **kwargs)
-        return RecordSet.from_dict(data) if data is not None else None
+        return RecordSet.from_dict(data['recordSet']) if data is not None else None
 
     def list_record_sets(self, zone_id, start_from=None, max_items=None, record_name_filter=None, **kwargs):
         """

tests/test_batch_change.py

@@ -16,7 +16,7 @@
 
 import responses
 from sampledata import forward_zone
-from vinyldns.record import RecordType
+from vinyldns.record import RecordType, AData
 from vinyldns.serdes import to_json_string
 from vinyldns.batch_change import AddRecordChange, DeleteRecordSetChange, BatchChange, BatchChangeRequest, \
     DeleteRecordSet, AddRecord, BatchChangeSummary, ListBatchChangeSummaries
@@ -39,15 +39,17 @@ def check_single_changes_are_same(a, b):
 
 
 def test_create_batch_change(mocked_responses, vinyldns_client):
-    ar = AddRecord('foo.bar.com', RecordType.A, 100, '1.2.3.4')
+    ar = AddRecord('foo.bar.com', RecordType.A, 100, AData('1.2.3.4'))
     drs = DeleteRecordSet('baz.bar.com', RecordType.A)
 
-    arc = AddRecordChange(forward_zone.id, forward_zone.name, 'foo', 'foo.bar.com', RecordType.A, '1.2.3.4',
-                          'Complete', 'id1', 'system-message', 'rchangeid1', 'rsid1')
+    arc = AddRecordChange(forward_zone.id, forward_zone.name, 'foo', 'foo.bar.com', RecordType.A, 200,
+                          AData('1.2.3.4'), 'Complete', 'id1', 'system-message', 'rchangeid1', 'rsid1')
+
     drc = DeleteRecordSetChange(forward_zone.id, forward_zone.name, 'baz', 'baz.bar.com', RecordType.A, 'Complete',
                                 'id2', 'system-message', 'rchangeid2', 'rsid2')
     bc = BatchChange('user-id', 'user-name', 'batch change test', datetime.utcnow(), [arc, drc],
                      'bcid', 'owner-group-id')
+
     mocked_responses.add(
         responses.POST, 'http://test.com/zones/batchrecordchanges',
         body=to_json_string(bc), status=200
@@ -69,8 +71,9 @@ def test_create_batch_change(mocked_responses, vinyldns_client):
 
 
 def test_get_batch_change(mocked_responses, vinyldns_client):
-    arc = AddRecordChange(forward_zone.id, forward_zone.name, 'foo', 'foo.bar.com', RecordType.A, '1.2.3.4',
-                          'Complete', 'id1', 'system-message', 'rchangeid1', 'rsid1')
+    arc = AddRecordChange(forward_zone.id, forward_zone.name, 'foo', 'foo.bar.com', RecordType.A, 200,
+                          AData('1.2.3.4'), 'Complete', 'id1', 'system-message', 'rchangeid1', 'rsid1')
+
     drc = DeleteRecordSetChange(forward_zone.id, forward_zone.name, 'baz', 'baz.bar.com', RecordType.A, 'Complete',
                                 'id2', 'system-message', 'rchangeid2', 'rsid2')
     bc = BatchChange('user-id', 'user-name', 'batch change test', datetime.utcnow(), [arc, drc],

tests/test_records.py

@@ -88,9 +88,10 @@ def test_delete_record_set(record_set, mocked_responses, vinyldns_client):
 def test_get_record_set(record_set, mocked_responses, vinyldns_client):
     rs = copy.deepcopy(record_set)
     rs.id = rs.name + 'id'
+    response = {'recordSet': rs}
     mocked_responses.add(
         responses.GET, 'http://test.com/zones/{0}/recordsets/{1}'.format(rs.zone_id, rs.id),
-        body=to_json_string(rs), status=200
+        body=to_json_string(response), status=200
     )
     r = vinyldns_client.get_record_set(rs.zone_id, rs.id)
     check_record_sets_are_equal(rs, r)