Refactored zone history retrieval, parsing and displaying code.

powerdnsadmin/routes/admin.py

@@ -107,66 +107,58 @@ def get_record_changes(del_rrset, add_rrset):
     return changeset
 
 
+def filter_rr_list_by_name_and_type(rrset, record_name, record_type):
+    return list(filter(lambda rr: rr['name'] == record_name and rr['type'] == record_type, rrset))
+
+
 # out_changes is a list of  HistoryRecordEntry objects in which we will append the new changes
 # a HistoryRecordEntry represents a pair of add_rrset and del_rrset
-def extract_changelogs_from_a_history_entry(out_changes, history_entry, change_num, record_name=None, record_type=None):
+def extract_changelogs_from_history(histories, record_name=None, record_type=None):
-    if history_entry.detail is None:
+    out_changes = []
-        return
 
-    if "add_rrsets" in history_entry.detail:
+    for entry in histories:
-        detail_dict = json.loads(history_entry.detail)
+        changes = []
+
+        if "add_rrsets" in entry.detail:
+            details = json.loads(entry.detail)
+            if not details['add_rrsets'] and not details['del_rrsets']:
+                continue
         else:  # not a record entry
-        return
+            continue
 
-    add_rrsets = detail_dict['add_rrsets']
+        # filter only the records with the specific record_name, record_type
-    del_rrsets = detail_dict['del_rrsets']
+        if record_name != None and record_type != None:
+            details['add_rrsets'] = list(filter_rr_list_by_name_and_type(details['add_rrsets'], record_name, record_type))
+            details['del_rrsets'] = list(filter_rr_list_by_name_and_type(details['del_rrsets'], record_name, record_type))
 
-    for add_rrset in add_rrsets:
+            if not details['add_rrsets'] and not details['del_rrsets']:
-        exists = False
+                continue
-        for del_rrset in del_rrsets:
-            if del_rrset['name'] == add_rrset['name'] and del_rrset['type'] == add_rrset['type']:
-                exists = True
-                if change_num not in out_changes:
-                    out_changes[change_num] = []
-                out_changes[change_num].append(HistoryRecordEntry(history_entry, del_rrset, add_rrset, "*"))
-                break
-        if not exists:  # this is a new record
-            if change_num not in out_changes:
-                out_changes[change_num] = []
-            out_changes[change_num].append(
-                HistoryRecordEntry(history_entry, {}, add_rrset, "+"))  # (add_rrset, del_rrset, change_type)
-    for del_rrset in del_rrsets:
-        exists = False
-        for add_rrset in add_rrsets:
-            if del_rrset['name'] == add_rrset['name'] and del_rrset['type'] == add_rrset['type']:
-                exists = True  # no need to add in the out_changes set
-                break
-        if not exists:  # this is a deletion
-            if change_num not in out_changes:
-                out_changes[change_num] = []
-            out_changes[change_num].append(HistoryRecordEntry(history_entry, del_rrset, {}, "-"))
 
-    # Sort them by the record name
+        # same record name and type RR are being deleted and created in same entry.
-    if change_num in out_changes:
+        del_add_changes = set([(r['name'], r['type']) for r in details['add_rrsets']]).intersection([(r['name'], r['type']) for r in details['del_rrsets']])
-        out_changes[change_num].sort(key=lambda change:
+        for del_add_change in del_add_changes:
-                change.del_rrset['name'] if change.del_rrset else change.add_rrset['name']
+            changes.append(HistoryRecordEntry(
+                entry,
+                filter_rr_list_by_name_and_type(details['del_rrsets'], del_add_change[0], del_add_change[1]).pop(0),
+                filter_rr_list_by_name_and_type(details['add_rrsets'], del_add_change[0], del_add_change[1]).pop(0),
+                "*")
             )
 
-    # only used for changelog per record
+        for rrset in details['add_rrsets']:
-    if record_name != None and record_type != None:  # then get only the records with the specific (record_name, record_type) tuple
+            if (rrset['name'], rrset['type']) not in del_add_changes:
-        if change_num in out_changes:
+                changes.append(HistoryRecordEntry(entry, {}, rrset, "+"))
-            changes_i = out_changes[change_num]
+
-        else:
+        for rrset in details['del_rrsets']:
-            return
+            if (rrset['name'], rrset['type']) not in del_add_changes:
-        for hre in changes_i:  # for each history record entry in changes_i
+                changes.append(HistoryRecordEntry(entry, rrset, {}, "-"))
-            if 'type' in hre.add_rrset and hre.add_rrset['name'] == record_name and hre.add_rrset[
+
-                'type'] == record_type:
+        # sort changes by the record name
-                continue
+        if changes:
-            elif 'type' in hre.del_rrset and hre.del_rrset['name'] == record_name and hre.del_rrset[
+            changes.sort(key=lambda change:
-                'type'] == record_type:
+                    change.del_rrset['name'] if change.del_rrset else change.add_rrset['name']
-                continue
+                    )
-            else:
+            out_changes.extend(changes)
-                out_changes[change_num].remove(hre)
+    return out_changes
 
 
 # records with same (name,type) are considered as a single HistoryRecordEntry
@@ -184,20 +176,15 @@ class HistoryRecordEntry:
         self.changed_fields = []  # contains a subset of : [ttl, name, type]
         self.changeSet = []  # all changes for the records of this add_rrset-del_rrset pair
 
-        if change_type == "+":  # addition
+        if change_type == "+" or change_type == "-":
             self.changed_fields.append("name")
             self.changed_fields.append("type")
             self.changed_fields.append("ttl")
-            self.changeSet = get_record_changes(del_rrset, add_rrset)
-        elif change_type == "-":  # removal
-            self.changed_fields.append("name")
-            self.changed_fields.append("type")
-            self.changed_fields.append("ttl")
-            self.changeSet = get_record_changes(del_rrset, add_rrset)
 
         elif change_type == "*":  # edit of unchanged
             if add_rrset['ttl'] != del_rrset['ttl']:
                 self.changed_fields.append("ttl")
+
         self.changeSet = get_record_changes(del_rrset, add_rrset)
 
     def toDict(self):
@@ -882,9 +869,7 @@ class DetailedHistory():
                                                        ip_address=detail_dict['ip_address'])
 
         elif 'add_rrsets' in detail_dict:  # this is a zone record change
-            # changes_set = []
             self.detailed_msg = ""
-            # extract_changelogs_from_a_history_entry(changes_set, history, 0)
 
         elif 'name' in detail_dict and 'template' in history.msg:  # template creation / deletion
             self.detailed_msg = render_template_string("""
@@ -997,20 +982,14 @@ class DetailedHistory():
 
 # convert a list of History objects into DetailedHistory objects
 def convert_histories(histories):
-    changes_set = dict()
     detailedHistories = []
-    j = 0
+    for history in histories:
-    for i in range(len(histories)):
+        if history.detail and ('add_rrsets' in history.detail or 'del_rrsets' in history.detail):
-        if histories[i].detail and ('add_rrsets' in histories[i].detail or 'del_rrsets' in histories[i].detail):
+            history_as_list = list()
-            extract_changelogs_from_a_history_entry(changes_set, histories[i], j)
+            history_as_list.append(history)
-            if j in changes_set:
+            detailedHistories.append(DetailedHistory(history, extract_changelogs_from_history(history_as_list)))
-                detailedHistories.append(DetailedHistory(histories[i], changes_set[j]))
-            else:  # no changes were found
-                detailedHistories.append(DetailedHistory(histories[i], None))
-            j += 1
-
         else:
-            detailedHistories.append(DetailedHistory(histories[i], None))
+            detailedHistories.append(DetailedHistory(history, None))
     return detailedHistories
 
 

powerdnsadmin/routes/domain.py

@@ -25,7 +25,7 @@ from ..models.domain_setting import DomainSetting
 from ..models.base import db
 from ..models.domain_user import DomainUser
 from ..models.account_user import AccountUser
-from .admin import extract_changelogs_from_a_history_entry
+from .admin import extract_changelogs_from_history
 from ..decorators import history_access_required
 domain_bp = Blueprint('domain',
                       __name__,
@@ -201,17 +201,6 @@ def changelog(domain_name):
     if not domain:
         abort(404)
 
-    # Query domain's rrsets from PowerDNS API
-    rrsets = Record().get_rrsets(domain.name)
-    current_app.logger.debug("Fetched rrsets: \n{}".format(pretty_json(rrsets)))
-
-    # API server might be down, misconfigured
-    if not rrsets and domain.type != 'Slave':
-        abort(500)
-
-    records_allow_to_edit = Setting().get_records_allow_to_edit()
-    records = []
-
     # get all changelogs for this domain, in descening order
     if current_user.role.name in [ 'Administrator', 'Operator' ]:
         histories = History.query.filter(History.domain_id == domain.id).order_by(History.created_on.desc()).all()
@@ -230,49 +219,13 @@ def changelog(domain_name):
                                 DomainUser.user_id == current_user.id,
                                 AccountUser.user_id == current_user.id
                         ),
-                        History.domain_id == domain.id
+                        History.domain_id == domain.id,
+                        History.detail.isnot(None)
                 )
             ).all()
 
-    if StrictVersion(Setting().get('pdns_version')) >= StrictVersion('4.0.0'):
+    changes_set = extract_changelogs_from_history(histories)
-        for r in rrsets:
-            if r['type'] in records_allow_to_edit:
-                r_name = r['name'].rstrip('.')
 
-                # If it is reverse zone and pretty_ipv6_ptr setting
-                # is enabled, we reformat the name for ipv6 records.
-                if Setting().get('pretty_ipv6_ptr') and r[
-                        'type'] == 'PTR' and 'ip6.arpa' in r_name and '*' not in r_name:
-                    r_name = dns.reversename.to_address(
-                        dns.name.from_text(r_name))
-
-                # Create the list of records in format that
-                # PDA jinja2 template can understand.
-                index = 0
-                for record in r['records']:
-                    if (len(r['comments'])>index):
-                        c=r['comments'][index]['content']
-                    else:
-                        c=''
-                    record_entry = RecordEntry(
-                        name=r_name,
-                        type=r['type'],
-                        status='Disabled' if record['disabled'] else 'Active',
-                        ttl=r['ttl'],
-                        data=record['content'],
-                        comment=c,
-                        is_allowed_edit=True)
-                    index += 1
-                    records.append(record_entry)
-    else:
-        # Unsupported version
-        abort(500)
-
-    changes_set = dict()
-    for i in range(len(histories)):
-        extract_changelogs_from_a_history_entry(changes_set, histories[i], i)
-        if i in changes_set and len(changes_set[i]) == 0: # if empty, then remove the key
-            changes_set.pop(i)
     return render_template('domain_changelog.html', domain=domain, allHistoryChanges=changes_set)
 
 """
@@ -289,17 +242,18 @@ def record_changelog(domain_name, record_name, record_type):
     domain = Domain.query.filter(Domain.name == domain_name).first()
     if not domain:
         abort(404)
-    # Query domain's rrsets from PowerDNS API
-    rrsets = Record().get_rrsets(domain.name)
-    current_app.logger.debug("Fetched rrsets: \n{}".format(pretty_json(rrsets)))
-
-    # API server might be down, misconfigured
-    if not rrsets and domain.type != 'Slave':
-        abort(500)
 
     # get all changelogs for this domain, in descening order
     if current_user.role.name in [ 'Administrator', 'Operator' ]:
-        histories = History.query.filter(History.domain_id == domain.id).order_by(History.created_on.desc()).all()
+        histories = History.query \
+            .filter(
+                db.and_(
+                        History.domain_id == domain.id,
+                        History.detail.like("%{}%".format(record_name))
+                )
+            ) \
+            .order_by(History.created_on.desc()) \
+            .all()
     else:
         # if the user isn't an administrator or operator, 
         # allow_user_view_history must be enabled to get here,
@@ -309,43 +263,23 @@ def record_changelog(domain_name, record_name, record_type):
             .outerjoin(DomainUser, Domain.id == DomainUser.domain_id) \
             .outerjoin(Account, Domain.account_id == Account.id) \
             .outerjoin(AccountUser, Account.id == AccountUser.account_id) \
-            .order_by(History.created_on.desc()) \
             .filter(
                 db.and_(db.or_(
                                 DomainUser.user_id == current_user.id,
                                 AccountUser.user_id == current_user.id
                         ), 
-                        History.domain_id == domain.id
+                        History.domain_id == domain.id,
+                        History.detail.like("%{}%".format(record_name))
                 )
-            ).all()
+            ) \
+            .order_by(History.created_on.desc()) \
+            .all()
 
-    changes_set_of_record = dict()
+    changes_set = extract_changelogs_from_history(histories, record_name, record_type)
-    for i in range(len(histories)):
-        extract_changelogs_from_a_history_entry(changes_set_of_record, histories[i], i, record_name, record_type)
-        if i in changes_set_of_record and len(changes_set_of_record[i]) == 0: # if empty, then remove the key
-            changes_set_of_record.pop(i)
 
-    indexes_to_pop = []
+    return render_template('domain_changelog.html', domain=domain, allHistoryChanges=changes_set,
-    for change_num in changes_set_of_record:
-        changes_i = changes_set_of_record[change_num]
-        for hre in changes_i: # for each history record entry in changes_i
-            if  'type' in hre.add_rrset and hre.add_rrset['name'] == record_name and hre.add_rrset['type'] == record_type:
-                continue
-            elif 'type' in hre.del_rrset and hre.del_rrset['name'] == record_name and hre.del_rrset['type'] == record_type:
-                continue
-            else:
-                changes_set_of_record[change_num].remove(hre)
-        if change_num in changes_set_of_record and len(changes_set_of_record[change_num]) == 0: # if empty, then remove the key
-            indexes_to_pop.append(change_num)
-    
-    for i in indexes_to_pop:
-        changes_set_of_record.pop(i)
-
-    return render_template('domain_changelog.html', domain=domain, allHistoryChanges=changes_set_of_record, 
                             record_name = record_name, record_type = record_type)
 
-
-
 @domain_bp.route('/add', methods=['GET', 'POST'])
 @login_required
 @can_create_domain

powerdnsadmin/templates/admin_history_table.html

@@ -10,26 +10,41 @@
   <table id="tbl_history" class="table table-bordered table-striped">
     <thead>
       <tr>
-        <th>Changed by</th>
-        <th>Content</th>
         <th>Time</th>
+        <th>Content</th>
+        <th>Changed by</th>
         <th>Detail</th>
       </tr>
     </thead>
     <tbody>
       {% for history in histories %}
         <tr class="odd gradeX">
-          <td>{{ history.history.created_by }}</td>
-          <td>{{ history.history.msg }}</td>
           <td>{{ history.history.created_on }}</td>
+          <td>{{ history.history.msg }}</td>
+          <td>{{ history.history.created_by }}</td>
           <td width="6%">
             <div id="history-info-div-{{ loop.index0 }}" style="display: none;">
               {{ history.detailed_msg | safe }}
               {% if history.change_set %}
                 <div class="content">
-                  <div id="change_index_definition"></div>
+                <table class="table table-bordered table-striped">
-                  {% call applied_change_macro.applied_change_template(history.change_set) %}
+                <thead><tr>
+                    <th>Name</th>
+                    <th>Type</th>
+                    <th>TTL</th>
+                    <th>Data</th>
+                    <th>Status</th>
+                    <th>Comment</th>
+                </tr></thead>
+                <tbody>
+				{% for applied_change in history.change_set %}
+                <tr>
+                    {% call applied_change_macro.applied_change_template(applied_change) %}
                     {% endcall %}
+                </tr>
+				{% endfor %}
+                </tbody>
+                </table>
                 </div>
               {% endif %}
             </div>
@@ -52,7 +67,7 @@
   $(document).ready(function () {
     table = $('#tbl_history').DataTable({
       "order": [
-        [2, "desc"]
+        [0, "desc"]
       ],
       "searching": true,
       "columnDefs": [{
@@ -60,7 +75,7 @@
         "render": function (data, type, row) {
           return moment.utc(data).local().format('YYYY-MM-DD HH:mm:ss');
         },
-        "targets": 2
+        "targets": 0
       }],
       "info": true,
       "autoWidth": false,

powerdnsadmin/templates/applied_change_macro.html

@@ -1,55 +1,82 @@
-{% macro applied_change_template(change_set) -%}
+{% macro applied_change_template(hist_rec_entry) -%}
 {{ caller() }}
-{% for hist_rec_entry in change_set %}
-<table id="tbl_records" class="table table-bordered">
-    <thead>
-        <tr>
-            <th scope="col" colspan="3">
-                {% if hist_rec_entry.change_type == '-' %}
-                    <span class="diff diff-deletion">{{
-                        hist_rec_entry.del_rrset['name'] }} {{ hist_rec_entry.del_rrset['type']
-                    }}</span>
-                {% elif hist_rec_entry.change_type == '+' %}
-                    <span class="diff diff-addition">{{
-                        hist_rec_entry.add_rrset['name'] }} {{ hist_rec_entry.add_rrset['type']
-                    }}</span>
-                {% else %}
-                    <span class="diff diff-unchanged">{{
-                        hist_rec_entry.add_rrset['name'] }} {{ hist_rec_entry.add_rrset['type']
-                    }}</span>
-                {% endif %}
 
-                , TTL
+<td>
-                {% if not 'ttl' in hist_rec_entry.changed_fields %}
+{% if hist_rec_entry.change_type == '-' %}
-                    <span class="diff diff-unchanged">{{
+    <div class="diff diff-deletion">{{
+        hist_rec_entry.del_rrset['name']
+    }}
+{% elif hist_rec_entry.change_type == '+' %}
+    <div class="diff diff-addition">{{
+        hist_rec_entry.add_rrset['name']
+    }}
+{% else %}
+    <div class="diff diff-unchanged">{{
+        hist_rec_entry.add_rrset['name']
+    }}
+{% endif %}
+    </div>
+</td>
+
+<td>
+{% if hist_rec_entry.change_type == '-' %}
+    <div class="diff diff-deletion">{{
+        hist_rec_entry.del_rrset['type']
+    }}
+{% elif hist_rec_entry.change_type == '+' %}
+    <div class="diff diff-addition">{{
+        hist_rec_entry.add_rrset['type']
+    }}
+{% else %}
+    <div class="diff diff-unchanged">{{
+        hist_rec_entry.add_rrset['type']
+    }}
+{% endif %}
+    </div>
+</td>
+
+<td>
+{% if not 'ttl' in hist_rec_entry.changed_fields %}
+    <div class="diff diff-unchanged">{{
         hist_rec_entry.add_rrset['ttl']
-                    }}</span>
+    }}</div>
-                {% else %}
+{% else %}
-                    {% if hist_rec_entry.change_type in ['-', '*'] %}
-                        <span class="diff diff-deletion">{{
-                            hist_rec_entry.del_rrset['ttl']
-                        }}</span>
-                    {% endif %}
     {% if hist_rec_entry.change_type in ['+', '*'] %}
-                        <span class="diff diff-addition">{{
+        <div class="diff diff-addition">{{
             hist_rec_entry.add_rrset['ttl']
-                        }}</span>
+        }}</div>
+    {% endif %}
+    {% if hist_rec_entry.change_type in ['-', '*'] %}
+        <div class="diff diff-deletion">{{
+            hist_rec_entry.del_rrset['ttl']
+        }}</div>
+    {% endif %}
+{% endif %}
+</td>
+
+<td style="word-break: break-all">
+{% for changes in hist_rec_entry.changeSet %}
+    {% if changes[2] == "unchanged" or (changes[2] == "edit" and changes[0]['content'] == changes[1]['content']) %}
+        <div class="diff diff-unchanged">{{
+            changes[1]['content']
+        }}</div>
+    {% else %}
+        {% if changes[2] in ["deletion", "edit"] %}
+            <div class="diff diff-deletion">{{
+                changes[0]['content']
+            }}</div>
+        {% endif %}
+        {% if changes[2] in ["addition", "edit"] %}
+            <div class="diff diff-addition">{{
+                changes[1]['content']
+            }}</div>
         {% endif %}
     {% endif %}
+{% endfor %}
+</td>
 
-            </th>
+<td>
-        </tr>
+{% for changes in hist_rec_entry.changeSet %}
-
-        <tr>
-            <th scope="col" style="width: 150px;">Status</th>
-            <th scope="col" style="width: 400px;">Data</th>
-            <th scope="col" style="width: 400px;">Comment</th>
-        </tr>
-    </thead>
-    <tbody>
-        {% for changes in hist_rec_entry.changeSet %}
-        <tr>
-            <td style="word-break: break-all">
     {% if changes[2] == "unchanged" or
         (changes[2] == "edit" and changes[0]['disabled'] == changes[1]['disabled']) %}
         <div class="diff diff-unchanged">{{
@@ -67,27 +94,11 @@
             }}</div>
         {% endif %}
     {% endif %}
-            </td>
+{% endfor %}
-            <td style="word-break: break-all">
+</td>
-                {% if changes[2] == "unchanged" or
+
-                        (changes[2] == "edit" and changes[0]['content'] == changes[1]['content']) %}
+<td style="word-break: break-all">
-                    <div class="diff diff-unchanged">{{
+{% for changes in hist_rec_entry.changeSet %}
-                        changes[1]['content']
-                    }}</div>
-                {% else %}
-                    {% if changes[2] in ["deletion", "edit"] %}
-                        <div class="diff diff-deletion">{{
-                            changes[0]['content']
-                        }}</div>
-                    {% endif %}
-                    {% if changes[2] in ["addition", "edit"] %}
-                        <div class="diff diff-addition">{{
-                            changes[1]['content']
-                        }}</div>
-                    {% endif %}
-                {% endif %}
-            </td>
-            <td style="word-break: break-all">
     {% if changes[2] == "unchanged" or
             (changes[2] == "edit" and changes[0]['comment'] == changes[1]['comment']) %}
         <div class="diff diff-unchanged">{{
@@ -105,10 +116,6 @@
             }}</div>
         {% endif %}
     {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-    </tbody>
-</table>
 {% endfor %}
+</td>
 {%- endmacro %}

powerdnsadmin/templates/domain_changelog.html

@@ -49,6 +49,12 @@
               <thead>
                 <tr>
                   <th>Changed on</th>
+                  <th>Name</th>
+                  <th>Type</th>
+                  <th>TTL</th>
+                  <th>Data</th>
+                  <th>Status</th>
+                  <th>Comment</th>
                   <th>Changed by</th>
                 </tr>
               </thead>
@@ -56,22 +62,14 @@
                 {% for applied_change in allHistoryChanges %}
                   <tr class="odd row_record" id="{{ domain.name }}">
                     <td id="changed_on" class="changed_on">
-                      {{ allHistoryChanges[applied_change][0].history_entry.created_on }}
+                      {{ applied_change.history_entry.created_on }}
                     </td>
-                    <td>
+                    {% call applied_change_macro.applied_change_template(applied_change) %}
-                      {{allHistoryChanges[applied_change][0].history_entry.created_by }}
-                    </td>
-                  </tr>
-                  <!-- Nested Table -->
-                  <tr style='visibility:collapse'>
-                    <td colspan="2">
-                      <div class="content">
-                        {% call applied_change_macro.applied_change_template(allHistoryChanges[applied_change]) %} 
                     {% endcall %}
-                      </div>
+                    <td>
+                      {{ applied_change.history_entry.created_by }}
                     </td>
                   </tr>
-                  <!-- end nested table -->
                 {% endfor %}
               </tbody>
             </table>
@@ -106,18 +104,6 @@
         });
     }
 
-    // handle click on history record
-    $(document.body).on("click", ".row_record", function (e) {
-        e.stopPropagation();
-        var nextRow = $(this).next('tr')
-        if (nextRow.css("visibility") == "visible")
-            nextRow.css("visibility", "collapse")
-        else
-            nextRow.css("visibility", "visible")
-
-    });
-
-
     var els = document.getElementsByClassName("changed_on");
     for (var i = 0; i < els.length; i++) {
         // els[i].innerHTML = moment.utc(els[i].innerHTML).local().format('YYYY-MM-DD HH:mm:ss');