Added typo data collection (/submitData)

sitescripts/urlfixer/web/submitData.py

 # coding: utf-8
 
 # This Source Code is subject to the terms of the Mozilla Public License
 # version 2.0 (the "License"). You can obtain a copy of the License at
 # http://mozilla.org/MPL/2.0/.
 
-# import MySQLdb, os, simplejson, sys, re
-# from sitescripts.utils import cached, get_config
-# from sitescripts.web import url_handler, basic_auth

[Wladimir Palant, 2012/10/09 11:51:14]

Any reason why the lines above are required? ;)

-
 import os, MySQLdb, simplejson as json
 from urlparse import parse_qs
-from math import sqrt

[Wladimir Palant, 2012/10/09 11:51:14]

I would prefer to just import math and use the function as math.sqrt() so that
it is obvious where the function came from. I've deviated from that rule quite a
bit myself in this repository - but I was learning the language as well.

 from sitescripts.web import url_handler
-from sitescripts.utils import get_config, setupStderr
-
-STATUS_TYPED = 1
-STATUS_TYPO = 2
-STATUS_CORRECTION = 3
-STATUS_FALSE_POSITIVE = 4
+from sitescripts.utils import cached, get_config, setupStderr
 
 @url_handler("/submitData")
 def submit_data(environ, start_response):
   setupStderr(environ['wsgi.errors'])
 
-  if environ['REQUEST_METHOD'].upper() != 'POST':
-    return showError('Unsupported request method', start_response)
-  
-  output = ""
+  if environ["REQUEST_METHOD"].upper() != "POST":
+    return showError("Unsupported request method", start_response)
   
   params = parse_qs(environ.get("QUERY_STRING", ""))
   requestVersion = params.get("version", ["0"])[0]
   data = "{}"
   try:
-      data_length = int(environ.get("CONTENT_LENGTH", "0"))
+    data_length = int(environ.get("CONTENT_LENGTH", "0"))
   except ValueError:
-      data_length = 0
+    data_length = 0
   if data_length != 0:
-      data = environ["wsgi.input"].read(data_length)
-      data = json.loads(data, ["{}"])

[Wladimir Palant, 2012/10/09 11:51:14]

Not sure I understand the significance of the second parameter - is that
supposed to be the encoding?

Also, this line should be outside the |if| block - if you fall back to the
default value then you should parse it as well.

Finally, you probably want to catch JSONDecodeError error and show an error
message - that's a user input error (not that a user should ever see it).

+    data = environ["wsgi.input"].read(data_length)
+  try:
+    data = json.loads(data)
+  except json.decoder.JSONDecodeError:
+    return showError("Error while parsing JSON data.", start_response)
   
-  try:
-    database = get_config().get("urlfixer", "database")
-    dbuser = get_config().get("urlfixer", "dbuser")
-    dbpasswd = get_config().get("urlfixer", "dbpassword")
-  except Exception:
-    return showError("Error while reading config.", start_response)

[Wladimir Palant, 2012/10/09 11:51:14]

That's a server configuration issue, not something that the user needs to know.
But we need to know about it which is why swallowing the error isn't a good
idea. I would rather suggest that you leave that error alone - it will get
written into the logs along with a precise description of what's wrong.

+  db = _get_db()
   
-  if os.name == "nt":
-    db = MySQLdb.connect(user=dbuser, passwd=dbpasswd, db=database,
-                           use_unicode=True, charset="utf8", named_pipe=True)
-  else:
-    db = MySQLdb.connect(user=dbuser, passwd=dbpasswd, db=database,
-                           use_unicode=True, charset="utf8")

[Wladimir Palant, 2012/10/09 11:51:14]

I suggest caching the database connection similarly to
sitescripts.crawler.web.crawler - we will get lots of connections here, not
opening a new connection each time is useful.

-  
-  for domain in data:

[Wladimir Palant, 2012/10/09 11:51:14]

I suggest:

  for domain, status in data.iteritems():

You can get both keys and values from the iterator.

-    process_domain(db, domain, data[domain])
+  for domain, status in data.iteritems():
+    process_domain(db, domain, status)
   
   db.commit()
-  db.close()

[Wladimir Palant, 2012/10/09 11:51:14]

Database shouldn't be closed if you share the database connection as I suggested
above.

   
-  response_headers = [('Content-type', 'text/plain'),
-                      ('Content-Length', str(len(output)))]

[Wladimir Palant, 2012/10/09 11:51:14]

Output is empty and this won't change (nobody can see it). Meaning: get rid of
the output variable, don't set "Content-Length" header (it will be set
automatically anyway), return an empty list.

+  response_headers = [("Content-type", "text/plain")]
   start_response("200 OK", response_headers)
-  return [output]
+  return []
 
 def process_domain(db, domain, status):
   domain_id = _get_domain_id(db, domain)
-  
   _increment_entry(db, domain_id, status)
-  
-  typed = _get_entry(db, domain_id, STATUS_TYPED)
-  typo = _get_entry(db, domain_id, STATUS_TYPO)
-  correction = _get_entry(db, domain_id, STATUS_CORRECTION)
-  false_positive = _get_entry(db, domain_id, STATUS_FALSE_POSITIVE)
-  
-  # Determine the occurences of the domain being correct as is
-  typed_count = get_weighted_count(typed)
-  # Determine the occurences of the domain as a typo
-  typo_count = get_weighted_count(typo)
-  # Determine the occurences of the domain as a correction
-  correction_count = get_weighted_count(correction)
-  # Determine the occurences of the domain as a false positive
-  false_positive_count = get_weighted_count(false_positive)
-  
-  # Determine the correctness of the domain and of the corrections (with a confidence level of 0.95)
-  # http://www.evanmiller.org/how-not-to-sort-by-average-rating.html
-  domain_correctness_score = calculateCorrectness(correction_count + typed_count, false_positive_count + typo_count)
-  correction_correctness_score = calculateCorrectness(correction_count, false_positive_count)
-  
-  _update_scores(db, domain_id, domain_correctness_score, correction_correctness_score)
-
-def calculateCorrectness(positive, negative):
-  score = 0
-  
-  if positive + negative > 0:
-    score = ((positive + 1.9208) / (positive + negative) - 
-      1.96 * sqrt((positive * negative) / (positive + negative) + 0.9604) / 
-      (positive + negative)) / (1 + 3.8416 / (positive + negative))
-  
-  return score
-
-def get_weighted_count(result):
-  if result == None:
-    return 0
-  
-  return result["curr_month"] * 0.4 + result["prev_month"] * 0.3 + result["curr_year"] * 0.2 + result["prev_year"] * 0.1
 
 def showError(message, start_response):
-  start_response('400 Processing Error', [('Content-Type', 'text/plain; charset=utf-8')])
-  return [message.encode('utf-8')]
+  start_response("400 Processing Error", [("Content-Type", "text/plain; charset=utf-8")])
+  return [message.encode("utf-8")]
+
+@cached(600)
+def _get_db():
+  database = get_config().get("urlfixer", "database")
+  dbuser = get_config().get("urlfixer", "dbuser")
+  dbpasswd = get_config().get("urlfixer", "dbpassword")
+  if os.name == "nt":
+    return MySQLdb.connect(user=dbuser, passwd=dbpasswd, db=database,
+                           use_unicode=True, charset="utf8", named_pipe=True)
+  else:
+    return MySQLdb.connect(user=dbuser, passwd=dbpasswd, db=database,
+                           use_unicode=True, charset="utf8")
 
 def _get_domain_id(db, domain):
   cursor = db.cursor(MySQLdb.cursors.DictCursor)
-  cursor.execute("SELECT id FROM domains WHERE domain='%s'" % (domain))

[Wladimir Palant, 2012/10/09 11:51:14]

Please don't ever generate queries dynamically, use prepared statements (keyword
if you want to know the reason: SQL injection). This should be:

    cursor.execute("SELECT id FROM domains WHERE domain = %s", domain)

And similarly for all the other SQL statements.

[Thomas Greiner, 2012/10/09 13:47:02]

Actually, I thought that this was the syntax for prepared statements. But I
guess I was wrong.

+  cursor.execute("SELECT id FROM domains WHERE domain = %s", (domain))
   result = cursor.fetchone()
   if result == None:
-    cursor.execute("INSERT INTO domains(domain) VALUES ('%s')" % (domain))
-    return _get_domain_id(db, domain)

[Wladimir Palant, 2012/10/09 11:51:14]

Should be:

  return db.insert_id()

+    cursor.execute("INSERT INTO domains(domain) VALUES (%s)", (domain))
+    return db.insert_id()
   else:
     return result["id"]
-
-def _get_entry(db, domain_id, status):
-  cursor = db.cursor(MySQLdb.cursors.DictCursor)
-  cursor.execute("SELECT * FROM corrections WHERE domain=%d AND status=%d" % (domain_id, status))
-  return cursor.fetchone()
   
 def _increment_entry(db, domain_id, status):
   cursor = db.cursor(MySQLdb.cursors.DictCursor)
-  cursor.execute("SELECT COUNT(*) AS count FROM corrections WHERE domain=%d AND status=%d" % (domain_id, status))
-  
-  if cursor.fetchone()["count"] < 1:
-    # Create and increment current period
-    cursor.execute("INSERT INTO corrections(domain, status, curr_month, prev_month, curr_year, prev_year) VALUES (%d, %d, %d, %d, %d, %d)" % (domain_id, status, 1, 0, 1, 0))
-  else:
-    # Increment current period
-    cursor.execute("UPDATE corrections SET curr_month=curr_month+1, curr_year=curr_year+1 WHERE domain=%d AND status=%d" % (domain_id, status))

[Wladimir Palant, 2012/10/09 11:51:14]

http://dev.mysql.com/doc/refman/5.0/en/insert-on-duplicate.html (see saveReport
in sitescripts.reports.utils for an example)

[Thomas Greiner, 2012/10/09 13:47:02]

Thanks for the tip!

-
-def _update_scores(db, domain_id, domain_correctness, correction_correctness):
-  cursor = db.cursor()
-  cursor.execute("UPDATE domains SET domain_correct=%f, correction_correct=%f WHERE id=%d" % (domain_correctness, correction_correctness, domain_id))
+  cursor.execute("INSERT INTO corrections(domain, status, curr_month, prev_month, curr_year, prev_year) VALUES (%s, %s, %s, %s, %s, %s) ON DUPLICATE KEY UPDATE curr_month = curr_month + 1, curr_year = curr_year + 1", (domain_id, status, 1, 0, 1, 0))