Issue 395 - Filter hits statistics backend

sitescripts/filterhits/web/submit.py

+# coding: utf-8
+
+# This file is part of the Adblock Plus web scripts,
+# Copyright (C) 2006-2015 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/>.
+
+import json
+import MySQLdb

[Sebastian Noack, 2015/03/31 07:55:21]

Nit: Third-party modules go between corelib and own module imports.

[kzar, 2015/03/31 09:48:56]

Done.

+import tempfile
+import time
+import traceback
+import os
+from datetime import datetime
+from errno import EEXIST
+
+from sitescripts.web import url_handler
+from sitescripts.utils import get_config, setupStderr
+from sitescripts.filterhits import db, geometrical_mean
+from sitescripts.filterhits.web import common
+
+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 = time.gmtime()
+
+  dir_name = time.strftime("%Y-%m-%d", now)
+  path = os.path.join(basepath, dir_name)
+  try:
+    os.makedirs(path)
+  except OSError as e:
+    if e.errno != EEXIST:
+      raise
+
+  with tempfile.NamedTemporaryFile(
+    prefix = str(int(time.mktime(now))) + "-",
+    suffix = ".log",
+    dir = path,
+    delete = False
+  ) as f:
+    print >> f, "[%s] %s" % (time.strftime('%d/%b/%Y:%H:%M:%S', now), query_string)
+    json.dump(data, f)
+    return f.name
+
+@url_handler("/submit")
+def submit(environ, start_response):
+  setupStderr(environ["wsgi.errors"])
+  config = get_config()
+
+  # Check that this is a POST request
+  if environ["REQUEST_METHOD"] != "POST":
+    return common.show_error("Unsupported request method", start_response)
+
+  # Parse the submitted JSON
+  try:
+    data = json.loads(environ["wsgi.input"].read(int(environ["CONTENT_LENGTH"])))
+  except (KeyError, IOError, ValueError):
+    return common.show_error("Error while parsing JSON data.", start_response)
+
+  # Make sure the submitted data was contained within an object at least
+  if not isinstance(data, dict):
+    return common.show_error("Error, data must be contained within an object.", start_response)
+
+  # Log the data to a file
+  log_dir = config.get("filterhitstats", "log_dir")
+  try:
+    log_file = log_filterhits(data, log_dir, environ.get("QUERY_STRING", ""))
+  except (OSError, IOError):
+    traceback.print_exc()
+    return common.show_error("Failed to write data to log file!", start_response,
+                             "500 Logging error")
+
+  # Update the geometrical_mean aggregations in the database
+  interval = config.get("filterhitstats", "interval")
+  try:
+    db_connection = db.connect()
+    try:
+      db.write(db_connection, geometrical_mean.update(interval, data))
+    finally:
+      db_connection.close()
+  except:
+    # Updating the aggregations in the database failed for whatever reason,
+    # log the details but continue to return 200 OK to the client to avoid
+    # re-transmission of the data.
+    processing_error_log = os.path.join(config.get("filterhitstats", "log_dir"),
+                                        "processing-errors.log")
+    with open(processing_error_log, "a+") as f:
+      message = "Problem processing data file %s:\n%s" % (
+        log_file, traceback.format_exc()
+      )
+      print >> f, "[%s] %s" % (datetime.now().strftime('%d/%b/%Y:%H:%M:%S %z'), message)
+
+  # Send back a 200 OK response
+  response_headers = [("Content-type", "text/plain")]

[Sebastian Noack, 2015/03/31 07:55:21]

Do we actually need to specify a content type for no content?

[kzar, 2015/03/31 09:48:56]

I guess not, I've removed it.

[Wladimir Palant, 2015/03/31 13:42:31]

Yes, we do - the browser still needs to know how to interpret the document, even
if it is empty.

[Sebastian Noack, 2015/03/31 13:46:53]

Well, if we would also set the correct status code - i.e. "204 No Content" - the
browser shouldn't expect any content anyway.

[kzar, 2015/04/01 19:09:39]

OK so I tested returning a 200 with and without the content type and both worked
for me, but if Wladimir says the browser still might need it I'm inclined to
believe him. You're right that 204 is technically a more correct status code but
my vote is to return 200 still as in practice maybe a client would check for
that exact code.

So I vote we just return 200 with the possibly redundant content type, as far as
I can see it is the most likely to work as we want and I don't think there are
any down sides apart from transmitting a few extra bytes.

+  start_response("200 OK", response_headers)
+  return []