Update using only one api call

Starting with the very first commit, the update was always done with
two api calls: one for DELETE and one for REPLACE. It is however
perfectly valid and save to do both at once, which makes it atomic, so
no need for the rollback. Plus it only updates the serial once.
There is no point in sending the full RRset data when deleting it, the
key attributes to identify it are enough. This also make the behaviour
consistent with the api docs [0] where it says "MUST NOT be included
when changetype is set to DELETE."

[0] https://doc.powerdns.com/authoritative/http-api/zone.html#rrset

powerdnsadmin/models/record.py

@@ -303,10 +303,47 @@ class Record(object):
                                   data=rrsets)
         return jdata
 
+    @staticmethod
+    def to_api_payload(new_rrsets, del_rrsets):
+        """Turn the given changes into a single api payload."""
+
+        def replace_for_api(rrset):
+            """Return a modified copy of the given RRset with changetype REPLACE."""
+            if not rrset or rrset.get('changetype', None) != 'REPLACE':
+                return rrset
+            replace_copy = dict(rrset)
+            # For compatibility with some backends: Remove comments from rrset if all are blank
+            if not any((bool(c.get('content', None)) for c in replace_copy.get('comments', []))):
+                replace_copy.pop('comments', None)
+            return replace_copy
+
+        def rrset_in(needle, haystack):
+            """Return whether the given RRset (identified by name and type) is in the list."""
+            for hay in haystack:
+                if needle['name'] == hay['name'] and needle['type'] == hay['type']:
+                    return True
+            return False
+
+        def delete_for_api(rrset):
+            """Return a minified copy of the given RRset with changetype DELETE."""
+            if not rrset or rrset.get('changetype', None) != 'DELETE':
+                return rrset
+            delete_copy = dict(rrset)
+            delete_copy.pop('ttl', None)
+            delete_copy.pop('records', None)
+            delete_copy.pop('comments', None)
+            return delete_copy
+
+        replaces = [replace_for_api(r) for r in new_rrsets]
+        deletes = [delete_for_api(r) for r in del_rrsets if not rrset_in(r, replaces)]
+        return {
+            'rrsets': replaces + deletes
+        }
+
     def apply(self, domain_name, submitted_records):
         """
         Apply record changes to a domain. This function
-        will make 2 calls to the PDNS API to DELETE and
+        will make 1 call to the PDNS API to DELETE and
         REPLACE records (rrsets)
         """
         current_app.logger.debug(
@@ -315,68 +352,24 @@ class Record(object):
         # Get the list of rrsets to be added and deleted
         new_rrsets, del_rrsets = self.compare(domain_name, submitted_records)
 
-        # Remove blank comments from rrsets for compatibility with some backends
+        # The history logic still needs *all* the deletes with full data to display a useful diff.
-        def remove_blank_comments(rrset):
+        # So create a "minified" copy for the api call, and return the original data back up
-            if not rrset['comments']:
+        api_payload = self.to_api_payload(new_rrsets['rrsets'], del_rrsets['rrsets'])
-                del rrset['comments']
+        current_app.logger.debug(f"api payload: \n{utils.pretty_json(api_payload)}")
-            elif isinstance(rrset['comments'], list):
-                # Merge all non-blank comment values into a list
-                merged_comments = [
-                    v
-                    for c in rrset['comments']
-                    for v in c.values()
-                    if v
-                ]
-                # Delete comment if all values are blank (len(merged_comments) == 0)
-                if not merged_comments:
-                    del rrset['comments']
-
-        for r in new_rrsets['rrsets']:
-            remove_blank_comments(r)
-
-        for r in del_rrsets['rrsets']:
-            remove_blank_comments(r)
 
         # Submit the changes to PDNS API
         try:
-            if del_rrsets["rrsets"]:
+            if api_payload["rrsets"]:
-                result = self.apply_rrsets(domain_name, del_rrsets)
+                result = self.apply_rrsets(domain_name, api_payload)
                 if 'error' in result.keys():
                     current_app.logger.error(
-                        'Cannot apply record changes with deleting rrsets step. PDNS error: {}'
+                        'Cannot apply record changes. PDNS error: {}'
                         .format(result['error']))
                     return {
                         'status': 'error',
                         'msg': result['error'].replace("'", "")
                     }
 
-            if new_rrsets["rrsets"]:
-                result = self.apply_rrsets(domain_name, new_rrsets)
-                if 'error' in result.keys():
-                    current_app.logger.error(
-                        'Cannot apply record changes with adding rrsets step. PDNS error: {}'
-                        .format(result['error']))
-
-                    # rollback - re-add the removed record if the adding operation is failed.
-                    if del_rrsets["rrsets"]:
-                        rollback_rrsets = del_rrsets
-                        for r in del_rrsets["rrsets"]:
-                            r['changetype'] = 'REPLACE'
-                        rollback = self.apply_rrsets(domain_name, rollback_rrsets)
-                        if 'error' in rollback.keys():
-                            return dict(status='error',
-                                        msg='Failed to apply changes. Cannot rollback previous failed operation: {}'
-                                        .format(rollback['error'].replace("'", "")))
-                        else:
-                            return dict(status='error',
-                                        msg='Failed to apply changes. Rolled back previous failed operation: {}'
-                                        .format(result['error'].replace("'", "")))
-                    else:
-                        return {
-                            'status': 'error',
-                            'msg': result['error'].replace("'", "")
-                        }
-
             self.auto_ptr(domain_name, new_rrsets, del_rrsets)
             self.update_db_serial(domain_name)
             current_app.logger.info('Record was applied successfully.')