From 1dba323cd529654268e8f48bf673e6b0b60b0500 Mon Sep 17 00:00:00 2001
From: Darshan Sanghani <dsanghani@ebaysf.com>
Date: Wed, 23 Jul 2014 14:58:15 -0700
Subject: [PATCH] Empty RecordSets can be re-provisioned

An empty recordset is just a logical entity with a given record type.
Right now we save the recordset name and type even when it is a null set,
thus preventing us from re-using the name as a different type (eg. A to
CNAME). It would be better to check for this empty recordset condition
and delete if possible, rather than throwing error 400.
This commit also adds CNAME type to Record and RecordSet fixtures
Closes-Bug: #1304088

Change-Id: I4dd0e7def848939d8f0053a5ca40b14a0b68b8b3
---
 designate/api/v1/records.py                   | 10 ++++
 designate/tests/__init__.py                   |  8 +++
 .../tests/test_api/test_v1/test_records.py    | 49 +++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/designate/api/v1/records.py b/designate/api/v1/records.py
index cd81d9c4d..0617e9531 100644
--- a/designate/api/v1/records.py
+++ b/designate/api/v1/records.py
@@ -203,6 +203,15 @@ def update_record(domain_id, record_id):
     return flask.jsonify(record_schema.filter(record))
 
 
+def _delete_recordset_if_empty(context, domain_id, recordset_id):
+    recordset = get_central_api().find_recordset(context, {
+        'id': recordset_id
+    })
+    # Make sure it's the right recordset
+    if len(recordset.records) == 0:
+        get_central_api().delete_recordset(context, domain_id, recordset_id)
+
+
 @blueprint.route('/domains/<uuid:domain_id>/records/<uuid:record_id>',
                  methods=['DELETE'])
 def delete_record(domain_id, record_id):
@@ -219,4 +228,5 @@ def delete_record(domain_id, record_id):
     get_central_api().delete_record(
         context, domain_id, record['recordset_id'], record_id)
 
+    _delete_recordset_if_empty(context, domain_id, record['recordset_id'])
     return flask.Response(status=200)
diff --git a/designate/tests/__init__.py b/designate/tests/__init__.py
index 4f68749c5..63329b74a 100644
--- a/designate/tests/__init__.py
+++ b/designate/tests/__init__.py
@@ -204,6 +204,10 @@ class TestCase(base.BaseTestCase):
             {'name': '_sip._tcp.%s', 'type': 'SRV'},
             {'name': '_sip._udp.%s', 'type': 'SRV'},
         ],
+        'CNAME': [
+            {'name': 'www.%s', 'type': 'CNAME'},
+            {'name': 'sub1.%s', 'type': 'CNAME'},
+        ]
     }
 
     record_fixtures = {
@@ -218,6 +222,10 @@ class TestCase(base.BaseTestCase):
         'SRV': [
             {'data': '0 5060 server1.example.org.', 'priority': 5},
             {'data': '1 5060 server2.example.org.', 'priority': 10},
+        ],
+        'CNAME': [
+            {'data': 'www.somedomain.org.'},
+            {'data': 'www.someotherdomain.com.'},
         ]
     }
 
diff --git a/designate/tests/test_api/test_v1/test_records.py b/designate/tests/test_api/test_v1/test_records.py
index 4e22b41ad..a91032bce 100644
--- a/designate/tests/test_api/test_v1/test_records.py
+++ b/designate/tests/test_api/test_v1/test_records.py
@@ -65,6 +65,55 @@ def test_create_record_existing_recordset(self):
         self.assertIn('name', response.json)
         self.assertEqual(response.json['name'], fixture['name'])
 
+    def test_create_record_name_reuse(self):
+        fixture_1 = self.get_record_fixture(self.recordset['type'])
+        fixture_1.update({
+            'name': self.recordset['name'],
+            'type': self.recordset['type'],
+        })
+
+        fixture_2 = self.get_record_fixture(self.recordset['type'], fixture=1)
+        fixture_2.update({
+            'name': self.recordset['name'],
+            'type': self.recordset['type'],
+        })
+
+        # Create 2 records
+        record_1 = self.post('domains/%s/records' % self.domain['id'],
+                             data=fixture_1)
+        record_2 = self.post('domains/%s/records' % self.domain['id'],
+                             data=fixture_2)
+
+        # Delete record 1, this should not have any side effects
+        self.delete('domains/%s/records/%s' % (self.domain['id'],
+                                               record_1.json['id']))
+
+        # Get the record 2 to ensure recordset did not get deleted
+        rec_2_get_response = self.get('domains/%s/records/%s' %
+                                     (self.domain['id'], record_2.json['id']))
+
+        self.assertIn('id', rec_2_get_response.json)
+        self.assertIn('name', rec_2_get_response.json)
+        self.assertEqual(rec_2_get_response.json['name'], fixture_1['name'])
+
+        # Delete record 2, this should delete the null recordset too
+        self.delete('domains/%s/records/%s' % (self.domain['id'],
+                                               record_2.json['id']))
+
+        # Re-create as a different type, but use the same name
+        fixture = self.get_record_fixture('CNAME')
+        fixture.update({
+            'name': self.recordset['name'],
+            'type': 'CNAME'
+        })
+
+        response = self.post('domains/%s/records' % self.domain['id'],
+                             data=fixture)
+
+        self.assertIn('id', response.json)
+        self.assertIn('name', response.json)
+        self.assertEqual(response.json['name'], fixture['name'])
+
     def test_create_record_junk(self):
         fixture = self.get_record_fixture(self.recordset['type'])
         fixture.update({