Issue 3176 - Add metadata to content blocker lists

sitescripts/content_blocker_lists/bin/generate_lists.py

Index: sitescripts/content_blocker_lists/bin/generate_lists.py
diff --git a/sitescripts/content_blocker_lists/bin/generate_lists.py b/sitescripts/content_blocker_lists/bin/generate_lists.py
index d86a52a95bc4edf0a14a017573c893f1f2c693bf..04b54f8bc2721989cafdd2ea0567b984c2feb330 100644
--- a/sitescripts/content_blocker_lists/bin/generate_lists.py
+++ b/sitescripts/content_blocker_lists/bin/generate_lists.py
@@ -16,61 +16,84 @@
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
+from collections import OrderedDict
+from contextlib import closing
+import json
 import os
 import subprocess
+import threading
+import time
+import re
 import urllib2
 
 from sitescripts.utils import get_config
 
-def _update_abp2blocklist():
+config = dict(get_config().items("content_blocker_lists"))
+
+def update_abp2blocklist():
   with open(os.devnull, "w") as devnull:
-    config = get_config()
-    abp2blocklist_path = config.get("content_blocker_lists",
-                                    "abp2blocklist_path")
+    abp2blocklist_path = config["abp2blocklist_path"]
     if os.path.isdir(abp2blocklist_path):
         subprocess.check_call(("hg", "pull", "-u", "-R", abp2blocklist_path),
                               stdout=devnull)
     else:
-      abp2blocklist_url = config.get("content_blocker_lists",
-                                     "abp2blocklist_url")
-      subprocess.check_call(("hg", "clone", abp2blocklist_url,
+      subprocess.check_call(("hg", "clone", config["abp2blocklist_url"],
                              abp2blocklist_path), stdout=devnull)
     subprocess.check_call(("npm", "install"), cwd=abp2blocklist_path,
                           stdout=devnull)
 
-def _download(url_key):
-  url = get_config().get("content_blocker_lists", url_key)
-  response = urllib2.urlopen(url)
+def parse_filter_list_header(filter_list):
+  body_start = re.search(r"^[^![]", filter_list, re.MULTILINE).start()
+  field_re = re.compile(r"^!\s*([^:\s]+):\s*(.+)$", re.MULTILINE)

[Sebastian Noack, 2015/12/07 12:38:24]

This logic can be simplified, in particular since we are only looking for the
version:

for match in re.finditer(r"^(?:([^![])|!\s*Version:\s*(.+)$)", filter_list,
re.MULTILINE):
  end_of_headers, version = match.groups()
  if end_of_headers:
    break
  if version:
    return version

[kzar, 2015/12/08 12:52:21]

IMHO that doesn't look easier to read. Also it seems to me that we should just
parse all the fields while at it.

[Felix Dahlke, 2015/12/08 13:39:51]

Reading both pieces of code for the first time, I consider Sebastian's variant
to be a bit easier to get, but the difference isn't huge. Considering how we
really don't need anything but the version, I'd lean towards that one.

[kzar, 2015/12/08 14:34:24]

OK I've gone with a slightly simplified version of your approach Sebastian.

+  return { match.group(1): match.group(2)
+           for match in field_re.finditer(filter_list, 0, body_start) }
+
+def download_filter_list(url):
+  with closing(urllib2.urlopen(url)) as response:
+    body = response.read()
+  header = parse_filter_list_header(body)
+  return body, url, header["Version"]
+
+def generate_metadata(filter_lists, expires):
+  metadata = OrderedDict((
+    ("version", time.strftime("%Y%m%d%H%M", time.gmtime())),
+    ("expires", expires),
+    ("sources", [])
+  ))
+  for body, url, version in filter_lists:
+    metadata["sources"].append({ "url": url, "version": version })

[Sebastian Noack, 2015/12/07 12:38:24]

See https://www.python.org/dev/peps/pep-0008/#pet-peeves

[kzar, 2015/12/08 12:52:21]

Done.

+  return metadata
+
+def pipe_in(process, filter_lists):
   try:
-    return response.read()
+    for body, _, _ in filter_lists:
+      print >>process.stdin, body
   finally:
-    response.close()
-
-def _convert_filter_list(sources, destination_path_key):
-  config = get_config()
-  destination_path = config.get("content_blocker_lists", destination_path_key)
-  with open(destination_path, "wb") as destination_file:
-    abp2blocklist_path = config.get("content_blocker_lists",
-                                    "abp2blocklist_path")
-    process = subprocess.Popen(("node", "abp2blocklist.js"),
-                               cwd=abp2blocklist_path, stdin=subprocess.PIPE,
-                               stdout=destination_file)
-    try:
-      for source in sources:
-        print >>process.stdin, source
-    finally:
-      process.stdin.close()
-      process.wait()
+    process.stdin.close()
+    process.wait()
 
+def write_block_list(filter_lists, path, expires):
+  block_list = generate_metadata(filter_lists, expires)
+  process = subprocess.Popen(("node", "abp2blocklist.js"),
+                             cwd=config["abp2blocklist_path"],
+                             stdin=subprocess.PIPE, stdout=subprocess.PIPE)
+  threading.Thread(target=pipe_in, args=(process, filter_lists)).start()
+  block_list["rules"] = json.load(process.stdout)
   if process.returncode:

[Sebastian Noack, 2015/12/07 12:38:24]

Note that returncode is set by the wait() method. So we should rather call it
here rather than in the thread to avoid race conditions.

[kzar, 2015/12/08 12:52:21]

Done.

     raise Exception("abp2blocklist returned %s" % process.returncode)
 
+  with open(path, "wb") as destination_file:
+    json.dump(block_list, destination_file, indent=2, separators=(",", ": "))
+
 if __name__ == "__main__":
-  _update_abp2blocklist()
+  update_abp2blocklist()
 
-  easylist = _download("easylist_url")
-  exceptionrules = _download("exceptionrules_url")
+  easylist = download_filter_list(config["easylist_url"])
+  exceptionrules = download_filter_list(config["exceptionrules_url"])
 
-  _convert_filter_list([easylist], "easylist_content_blocker_path")
-  _convert_filter_list([easylist, exceptionrules],
-                       "combined_content_blocker_path")
+  write_block_list([easylist],
+                   config["easylist_content_blocker_path"],
+                   config["easylist_content_blocker_expires"])
+  write_block_list([easylist, exceptionrules],
+                   config["combined_content_blocker_path"],
+                   config["combined_content_blocker_expires"])