Issue 6685 - Offer incremental filter list downloads

abp/filters/renderer.py

 # This file is part of Adblock Plus <https://adblockplus.org/>,
 # Copyright (C) 2006-present eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ skipping 106 matching lines @@
     """Insert metadata comment with version (a.k.a. date)."""
     first_line, rest = _first_and_rest(lines)
     version = Metadata('Version', time.strftime('%Y%m%d%H%M', time.gmtime()))
     return itertools.chain([first_line, version], rest)
 
 
 def _remove_duplicates(lines):
     """Remove duplicate metadata and headers."""
     # Always remove checksum -- a checksum coming from a fragment
     # will not match for the rendered list.
-    seen = {'Checksum'}

[Sebastian Noack, 2018/08/28 19:52:18]

Since we parse metadata with arbitrary keys now, we have to account for keys
with inconsistent capitalization, here:

  seen = {'checksum'}
  for i, line in enumerate(lines):
      if line.type == 'metadata':
          key = line.key.lower()
          if key not in seen:
              seen.add(key)
              yield line

[rhowell, 2018/08/29 21:43:34]

Done.

+    seen = {'checksum'}
     for i, line in enumerate(lines):
         if line.type == 'metadata':
-            if line.key not in seen:
-                seen.add(line.key)
+            key = line.key.lower()
+            if key not in seen:
+                seen.add(key)
                 yield line
         elif line.type == 'header':
             if i == 0:
                 yield line
         else:
             yield line
 
 
 def _validate(lines):
     """Validate the final list."""
@@ skipping 34 matching lines @@
     _logger.info('Rendering: %s', name)
     lines, default_source = _get_and_parse_fragment(name, sources, top_source)
     lines = _process_includes(sources, default_source, [name], lines)
     for proc in [_process_timestamps, _insert_version, _remove_duplicates,
                  _validate]:
         lines = proc(lines)
     return lines
 
 
 def _split_list_for_diff(list_in):
-    filterlist, metadata, keys = set(), set(), set()
+    """Split a filter list into metadata, keys, and rules."""

[Vasily Kuznetsov, 2018/08/31 12:11:02]

Nit: strictly speaking this function now returns metadata and rules (the keys
are just part of metadata). The docstring could be updated.

[rhowell, 2018/08/31 21:21:02]

Ah, good point! I'll update that.

+    metadata = {}
+    rules = set()
     for line in parse_filterlist(list_in):
-        if line.type == 'metadata' and 'Checksum' not in line.to_string():

[Sebastian Noack, 2018/08/28 19:52:18]

I think it has been agreed on that this isn't necessary, as any diff-aware
client ignores checksums anyway (besides most filter list if not all wouldn't
have a checksum anymore anyway).

[rhowell, 2018/08/29 21:43:35]

Done.

-            metadata.add(line.to_string())
-            keys.add(line.key)

[Sebastian Noack, 2018/08/28 19:52:18]

This code has to be changed as well, to handle case-insensitive keys:

  def _split_list_for_diff(list_in):
      metadata = {}
      for line in parse_filterlist(list_in):
          if line.type == 'metadata':
              metadata[line.key.lower()] = line
      ...

  def render_diff(base, latest):
      ...
      for key, line in latest_md.items():
          other = base_md.get(key)
          if not other or other.value != line.value:
              yield line.to_string()
      for key in set(base_md) - set(latest_md):
          yield '! {}:'.format(base_md[key].key)
      ...

[rhowell, 2018/08/29 21:43:34]

Any reason to use a dict instead of a set? I guess one advantage would be: if a
metadata item is removed in the new list, we would convert it to lower-case and
give it a blank value. If that's a concern (converting it to lower case), then I
can use dicts instead of sets, as you suggested.

[Sebastian Noack, 2018/08/29 21:56:37]

The algorithm I suggest here, is the (probably) simplest way to treat keys
case-insensitive while preserving the original capitalization. With a set alone
you cannot associate the normalized keys with the original.

[rhowell, 2018/08/29 22:52:23]

Done.

+        if line.type == 'metadata':
+            metadata[line.key.lower()] = line
         elif line.type == 'filter':
-            filterlist.add(line.to_string())
-    return filterlist, metadata, keys
+            rules.add(line.to_string())
+    return metadata, rules
 
 
 def render_diff(base, latest):
     """Return a diff between two filter lists.
 
     Parameters
     ----------
     base : iterator of str
         The base (old) list that we want to update to latest.
     lastest : iterator  of str
         The latest (most recent) list that we want to update to.
 
     Returns
     -------
     iterable of str
         A diff between two lists (https://issues.adblockplus.org/ticket/6685)
 
     """
-    latest_fl, latest_md, latest_keys = _split_list_for_diff(latest)
-    base_fl, base_md, base_keys = _split_list_for_diff(base)
-
-    new_md = latest_md - base_md
-    removed_keys = base_keys - latest_keys
-    add_fl = latest_fl - base_fl
-    remove_fl = base_fl - latest_fl
+    latest_metadata, latest_rules = _split_list_for_diff(latest)
+    base_metadata, base_rules = _split_list_for_diff(base)
 
     yield '[Adblock Plus Diff]'
-    for item in new_md:
-        yield item
-    for key in removed_keys:
-        # If a special comment has been removed, enter it with a blank value
-        # so the client will set it back to the default value
-        yield '! {}:'.format(key)
-    for item in remove_fl:
-        yield '- {}'.format(item)
-    for item in add_fl:
-        yield '+ {}'.format(item)
+    for key, latest in latest_metadata.items():
+        base = base_metadata.get(key)
+        if not base or base.value != latest.value:
+            yield latest.to_string()
+    for key in set(base_metadata) - set(latest_metadata):
+        yield '! {}:'.format(base_metadata[key].key)
+    for rule in base_rules - latest_rules:
+        yield '- {}'.format(rule)
+    for rule in latest_rules - base_rules:
+        yield '+ {}'.format(rule)