Issue 395 - Filter hits statistics backend

sitescripts/filterhits/common.py

Index: sitescripts/filterhits/common.py
diff --git a/sitescripts/filterhits/common.py b/sitescripts/filterhits/common.py
new file mode 100644
index 0000000000000000000000000000000000000000..8c3a651cd032c8e108de370141fbecd11000cf7e
--- /dev/null
+++ b/sitescripts/filterhits/common.py
@@ -0,0 +1,80 @@
+# coding: utf-8
+
+# This file is part of the Adblock Plus web scripts,
+# Copyright (C) 2006-2014 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
+# GNU General Public License for more details.
+#
+# 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 datetime import datetime
+import json, tempfile, os
+
+def showError(message, start_response, status="400 Processing Error"):
+  start_response(status, [("Content-Type", "text/plain; charset=utf-8")])
+  return [message.encode("utf-8")]
+
+def valid_filter_hit(filter_hit):
+  return (
+    isinstance(filter_hit, dict) and
+    "thirdParty" in filter_hit and
+    isinstance(filter_hit["thirdParty"], dict) and
+    "firstParty" in filter_hit and
+    isinstance(filter_hit["firstParty"], dict) and
+    "subscriptions" in filter_hit and
+    isinstance(filter_hit["subscriptions"], (list, tuple))
+  )
+
+def valid_log_data(data):
+  """
+    This returns True if the filterhits data passed is structured
+    roughly OK. Used as a quick check, it's not comprehensive.
+  """
+  return (

[Sebastian Noack, 2015/02/11 16:00:12]

I wonder whether we should rather properly check for exceptions than having
logic duplicated in a (currently incomplete) validation function. Same above.

[kzar, 2015/02/17 10:52:24]

Done.

+    isinstance(data, dict) and
+    "version" in data and
+    "timeSincePush" in data and
+    "addonName" in data and
+    "addonVersion" in data and
+    "application" in data and
+    "applicationVersion" in data and
+    "platform" in data and
+    "platformVersion" in data and
+    "filters" in data and
+    isinstance(data["filters"], dict) and
+    (not len(data["filters"]) or
+     valid_filter_hit(data["filters"].itervalues().next()))
+  )
+
+def datetime_to_timestamp(dt):
+  return int((dt - datetime(1970, 1, 1)).total_seconds())

[Sebastian Noack, 2015/02/11 16:00:12]

Hardcoding the epoch looks like a hack to me. Did you know that you can actually
convert a datetime to a timetuple and that to a UNIX timestamp:

time.mktime(dt.timetuple())

[kzar, 2015/02/17 10:52:24]

Done.

+
+def log_filterhits(data, basepath, query_string):
+  """
+    This logs the provided filterhits data as JSON to a file named after
+    the current timestamp in a directory named after the current date.
+  """
+  now = datetime.now()
+
+  dir_name = now.strftime("%Y-%m-%d")
+  path = os.path.join(basepath, dir_name)
+  if not os.path.exists(path):

[Sebastian Noack, 2015/02/11 16:00:12]

I'd rather catch the OSError, checking for e.errno == errno.EEXIST, rather than
checking whether the directory exists in advance, to eliminate race conditions
and reduce syscalls.

[kzar, 2015/02/17 10:52:24]

Done.

+    os.makedirs(path)
+
+  with tempfile.NamedTemporaryFile(
+    prefix = str(datetime_to_timestamp(now)) + "-",
+    suffix = ".log",
+    dir = path,
+    delete = False
+  ) as f:
+    f.write("[%s] \"%s\" %s\n" % (now.strftime('%d/%b/%Y:%H:%M:%S %z'),
+                                  query_string, json.dumps(data)))
+    return f.name