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..5523c7aa97811717e0531c2016556a239755c8ed 100644
--- a/sitescripts/content_blocker_lists/bin/generate_lists.py
+++ b/sitescripts/content_blocker_lists/bin/generate_lists.py
@@ -16,61 +16,78 @@
 # 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 download_filter_list(url):
+  with closing(urllib2.urlopen(url)) as response:
+    body = response.read()
+  version = re.search(r"^(?:[^[!])|!\s*Version:\s*(.+)$",

[Sebastian Noack, 2015/12/08 15:08:38]

Indeed, we can simply use re.search instead iterating over all matches.

However, note that with the regexp you use here begin of string is only matched
for the pattern before the alternation. So for example you would also match "!
foo ! Version: foo".

Also note that re.search() will return None, and therefore the .group() causes
an exception, if there is neither a Version header nor some filters in the list.

So this should rather be:

  match = re.search(r"^(?:(?:[^[!])|!\s*Version:\s*(.+)$)", body, re.MULTILINE)
  version = match and match.group(1)

[kzar, 2015/12/08 15:37:43]

Well it's true about the "! foo ! Version: foo" match, we're not expecting that
to happen but I guess checking for it can't harm. Done.

As for the other point, I don't think it's true that we're expecting the match
to be None. re.search() is returning a match for the version, or a match for a
line not starting with [ or !. In the latter case there is still a match object,
but .group(1) simply returns None as no capturing groups matched.

So the end result is we either get the version number or None, and the filter
list after the header is not searched.

[Sebastian Noack, 2015/12/08 15:45:51]

Well, theoretically the filter list can be empty. But it's probably not
important. LGTM.

+                      body, re.MULTILINE).group(1)
+  return body, url, 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})
+  return metadata
+
+def pipe_in(process, filter_lists):
   try:
-    return response.read()
+    for body, _, _ in filter_lists:
+      print >>process.stdin, body
   finally:
-    response.close()
+    process.stdin.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()
-
-  if process.returncode:
+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.wait():
     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"])