launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

Данило Шеган has proposed merging lp:~danilo/launchpad/bug-728220 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~danilo/launchpad/bug-728220/+merge/71343

= Bug 728220 =

Filter out any quoted strings from queries executed on the session DB when storing OOPS reports.

== Tests ==

bin/test -cvvt test_session_queries_filtered -t test_filter_session_statement

== Demo and Q/A ==

Generate an OOPS and wait for it to be synced, and then look through it and watch for session SQL statements.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/webapp/tests/test_errorlog.py
  lib/canonical/launchpad/webapp/errorlog.py
-- 
https://code.launchpad.net/~danilo/launchpad/bug-728220/+merge/71343
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-728220 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py	2011-08-10 06:50:52 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py	2011-08-12 11:38:25 +0000
@@ -13,7 +13,6 @@
 import logging
 import os
 import re
-import rfc822
 import stat
 import types
 import urllib
@@ -44,6 +43,7 @@
     IUnloggedException,
     )
 from canonical.launchpad.webapp.opstats import OpStats
+from canonical.launchpad.webapp.pgsession import PGSessionBase
 from canonical.launchpad.webapp.vhosts import allvhosts
 from canonical.lazr.utils import safe_hasattr
 from lp.app import versioninfo
@@ -157,6 +157,13 @@
     def __repr__(self):
         return '<ErrorReport %s %s: %s>' % (self.id, self.type, self.value)
 
+    def filter_session_statement(self, database_id, statement):
+        """Replace quoted strings with '%s' in statements on session DB."""
+        if database_id == 'SQL-' + PGSessionBase.store_name:
+            return re.sub("'[^']*'", "'%s'", statement)
+        else:
+            return statement
+
     def get_chunks(self):
         """Returns a list of bytestrings making up the oops disk content."""
         chunks = []
@@ -180,6 +187,7 @@
                                   urllib.quote(value, safe_chars)))
         chunks.append('\n')
         for (start, end, database_id, statement) in self.db_statements:
+            statement = self.filter_session_statement(database_id, statement)
             chunks.append('%05d-%05d@%s %s\n' % (
                 start, end, database_id, _normalise_whitespace(statement)))
         chunks.append('\n')

=== modified file 'lib/canonical/launchpad/webapp/tests/test_errorlog.py'
--- lib/canonical/launchpad/webapp/tests/test_errorlog.py	2011-08-10 06:50:52 +0000
+++ lib/canonical/launchpad/webapp/tests/test_errorlog.py	2011-08-12 11:38:25 +0000
@@ -102,6 +102,40 @@
             entry.db_statements[1],
             (5, 10, 'store_b', 'SELECT 2'))
 
+    def test_filter_session_statement(self):
+        """Removes quoted strings if database_id is SQL-session."""
+        entry = ErrorReport('OOPS-A0001', '', '', datetime.datetime.now(),
+                            '', '', '', '', 42, [], [], False)
+
+        statement = "SELECT 'gone'"
+        self.assertEqual(
+            "SELECT '%s'",
+            entry.filter_session_statement('SQL-session', statement))
+
+    def test_filter_session_statement_noop(self):
+        """If database_id is not SQL-session, it's a no-op."""
+        entry = ErrorReport('OOPS-A0001', '', '', datetime.datetime.now(), '',
+                            '', '', '', 42, [], [], False)
+
+        statement = "SELECT 'gone'"
+        self.assertEqual(
+            statement,
+            entry.filter_session_statement('SQL-launchpad', statement))
+
+    def test_session_queries_filtered(self):
+        """Test that session queries are filtered."""
+        entry = ErrorReport('OOPS-A0001', '', '', datetime.datetime.now(),
+                            '', '', '', '', 42, [],
+                            [(1, 5, 'store_a', "SELECT 'stays'"),
+                             (5, 10, 'SQL-session', "SELECT 'gone'")], False)
+        # Skip unrelated chunks and just get the SQL queries.
+        sql_chunks = entry.get_chunks()[13:-2]
+
+        self.assertEqual(
+            ["00001-00005@store_a SELECT 'stays'\n",
+             "00005-00010@SQL-session SELECT '%s'\n"],
+            sql_chunks)
+
     def test_write(self):
         """Test ErrorReport.write()"""
         entry = ErrorReport('OOPS-A0001', 'NotFound', 'error message',