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-2014 Eyeo GmbH
+# 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 MySQLdb, json, os
 from datetime import datetime
 from sitescripts.web import url_handler
 from sitescripts.utils import get_config, setupStderr
 
 import sitescripts.filterhits.common as common
 import sitescripts.filterhits.db as db
 import sitescripts.filterhits.geometrical_mean as geometrical_mean
 
 @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"].upper() != "POST":
+  if environ["REQUEST_METHOD"] != "POST":
     return common.showError("Unsupported request method", start_response)
 
   # Parse the submitted JSON
   data = "{}"
   try:
-    data_length = int(environ.get("CONTENT_LENGTH", "0"))
-  except ValueError:
+    data_length = int(environ["CONTENT_LENGTH"])
+  except (ValueError, KeyError):
     data_length = 0
   if data_length != 0:
     data = environ["wsgi.input"].read(data_length)
   try:
     data = json.loads(data)

[Sebastian Noack, 2015/02/17 14:59:17]

It seems that parsing an empty dictionary is unnecessary. Why don't you just use
|data = {}| here?

[Sebastian Noack, 2015/02/17 17:09:47]

data = {}
try:
  data_length = int(environ["CONTENT_LENGTH"])
except (ValueError, KeyError):
  pass
else:
  if data_length > 0:
    try:
      data = json.loads(environ["wsgi.input"].read(data_length))
    except ValueError:
      return common.showError("Error while parsing JSON data.", start_response)

Or if we just fail in case the request is empty, it can get even simpler:

try:
  data = json.loads(environ["wsgi.input"].read(int(environ["CONTENT_LENGTH"])))
except (ValueError, KeyError):
  return common.showError("Error while parsing JSON data.", start_response)

[Sebastian Noack, 2015/02/17 17:16:57]

Possibly you could also just use json.load():

try:
  data = json.load(environ["wsgi.input"])
except (ValueError, IOError):
  return common.showError("Error while parsing JSON data.", start_response)

[kzar, 2015/02/24 18:05:11]

Done.

   except json.decoder.JSONDecodeError:
     return common.showError("Error while parsing JSON data.", start_response)
 
-  # Make sure it looks roughly valid
-  if not common.valid_log_data(data):
-    return common.showError("Data looks invalid.", start_response)
-
   # Log the data to a file
   log_dir = config.get("filterhitstats", "log_dir")
   try:
     log_file = common.log_filterhits(data, log_dir,
                                      environ.get("QUERY_STRING", ""))
-  except OSError, IOError:
+  except (OSError, IOError):
     return common.showError("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.connect(config.get("filterhitstats", "dbuser"),
-               config.get("filterhitstats", "dbpassword"),
-               config.get("filterhitstats", "database"))
-    db.write(geometrical_mean.update(interval, data))
+    db_connection = db.connect(config.get("filterhitstats", "dbuser"),
+                               config.get("filterhitstats", "dbpassword"),
+                               config.get("filterhitstats", "database"))
+    db.write(db_connection, geometrical_mean.update(interval, data))
   except MySQLdb.Error, e:
     # 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.
     mysql_error_log = os.path.join(config.get("filterhitstats", "log_dir"),
                                    "mysql-errors.log")
     with open(mysql_error_log, "a+") as f:
       f.write("[%s] MySQL error (%d) when processing data file %s: \"%s\"\n" % (
         datetime.now().strftime('%d/%b/%Y:%H:%M:%S %z'),
         e.args[0], log_file, e.args[1]
       ))
   finally:
-    db.disconnect()
+    if db_connection:

[Sebastian Noack, 2015/02/17 14:59:17]

This will result in a NameError, in case db_connection failed to connect. You
need following pattern here:

try:
  connection = db.connect(...)
  try:
    ...
  finally:
    connection.close()
except MySQLdb.Error:
  ...

[kzar, 2015/02/24 18:05:11]

Done.

+      db_connection.close()
 
   # Send back a 200 OK response
   response_headers = [("Content-type", "text/plain")]
   start_response("200 OK", response_headers)
   return []