Issue 395 - Filter hits statistics backend

sitescripts/filterhits/geometrical_mean.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 itertools
-import sitescripts.filterhits.db as db
-from functools import partial
 
-def update_sql(interval, filter, domain, hits, timestamp):
+from sitescripts.filterhits import db
+
+def update(interval, data):
   """
-    Returns the SQL required to insert / update the hits for a
-    given domain + filter combo and to insert the filter if required.
+  Generator that provides all the SQL and parameters needed to update the
+  aggregations for the given data + interval in the database.
   """
-  filter = db.escape(filter)

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

Please don't manually escape or format SQL. Pass the variable parameters to the
cursor's execute() method instead, like:

cursor.execute("SELECT * FROM t WHERE foo = %s", ("bar",))

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

Done.

+  for filter, filter_data in data["filters"].iteritems():
+    yield ("""INSERT IGNORE INTO `filters`
+              (filter, sha1) VALUES (%s, UNHEX(SHA1(filter)))""", filter)
 
-  return (
-    ("INSERT IGNORE INTO `filters` " +
-     "(filter, md5) VALUES ('%s', UNHEX(MD5(filter)));" +

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

MD5 is deprecate due to known collision issues. You should use SHA1 instead.

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

Done.

-     "INSERT INTO `geometrical_mean` " +
-     "(filter_md5, domain, hits, timestamp) " +
-     "VALUES (UNHEX(MD5('%s')), '%s', %d, FROM_UNIXTIME(%d)) " +

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

It seems that timezone information are ignored here. I highly recommend to
convert all timestamps to UTC before storing/processing them, to avoid
inconsistencies and bugs.

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

As long as the client sends the timestamp properly as UTC we should be OK. I've
added a note to the ticket and review for the client side code that we expect
this.

-     "ON DUPLICATE KEY UPDATE " +
-     "hits = (" +
-     " POW(hits, 1 - (UNIX_TIMESTAMP(VALUES(timestamp)) - " +
-     "                UNIX_TIMESTAMP(timestamp)) / %d) * " +
-     " POW(VALUES(hits), (UNIX_TIMESTAMP(VALUES(timestamp)) - " +
-     "                    UNIX_TIMESTAMP(timestamp)) / %d)), " +
-     "timestamp = VALUES(timestamp);") % (
-       filter,
-       filter, db.escape(domain), int(hits), int(timestamp),
-       int(interval), int(interval)))
-
-def filter_hits(data):
-  """
-    Generator that provides all filter hits for the given data,
-    in tuples like (filter, domain, hits, latest).
-  """
-  for filter, filter_data in data['filters'].iteritems():
     domains = itertools.chain(filter_data.get("thirdParty", {}).iteritems(),
                               filter_data.get("firstParty", {}).iteritems())
     for domain, domain_data in domains:
-      yield (filter, domain, domain_data["hits"], domain_data["latest"] / 1000)
-
-def update(interval, data):
-  """
-    Returns an iterator of all the SQL statements needed to
-    update the aggregations for the given data + interval.
-  """
-  return itertools.imap(lambda fields: apply(partial(update_sql, interval), fields),

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

apply() is a Python 2.2 (?) relic before the *-syntax for variable arguments
were introduced. In modern Python you can just write following code:

itertools.imap(lambda fields: update_sql(interval, *fields), filter_hits(data))

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

Done.

-                        filter_hits(data))
+      yield ("""INSERT INTO `frequencies`
+                (filter_sha1, domain, frequency, timestamp)
+                VALUES (UNHEX(SHA1(%s)), %s, %s, FROM_UNIXTIME(%s))
+                ON DUPLICATE KEY UPDATE
+                frequency = (
+                  POW(frequency, 1 - (UNIX_TIMESTAMP(VALUES(timestamp)) -
+                                      UNIX_TIMESTAMP(timestamp)) / %s) *
+                  POW(VALUES(frequency), (UNIX_TIMESTAMP(VALUES(timestamp)) -
+                                          UNIX_TIMESTAMP(timestamp)) / %s)),
+                timestamp = VALUES(timestamp)""",
+             filter, domain, domain_data["hits"],
+             int(domain_data["latest"] / 1000), interval, interval)