Merge lp:~matsubara/python-oops-datedir-repo/833857-blankline-reqvars into lp:python-oops-datedir-repo

[Diogo Matsubara]

This branch fixes the serializer to correctly parse request variables with a blank line between them as found in some ISD oops reports.

[lp:~matsubara/python-oops-datedir-repo/833857-blankline-reqvars]

8. By Diogo Matsubara on 2011-08-25

fix bug 833857 (serializer can't read isd oops report file)

[Robert Collins (lifeless)]

The fact lines is an iterator is eyewatering, but an existing thing. Thanks.

review: Approve

=== modified file 'oops_datedir_repo/serializer_rfc822.py'
--- oops_datedir_repo/serializer_rfc822.py	2011-08-25 18:23:30 +0000
+++ oops_datedir_repo/serializer_rfc822.py	2011-08-25 20:39:09 +0000
@@ -93,33 +93,42 @@
     # support iteration.
     lines = iter(msg.fp)
 
-    # Request variables until the first blank line.
+    def is_req_var(line):
+        return "=" in line
+
+    def is_statement(line):
+        return re.match(r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line)
+
+    def is_traceback(line):
+        return 'traceback' in line.lower() or '== EXTRA DATA ==' in line
+
     req_vars = []
-    for line in lines:
-        line = line.strip()
-        if line == '':
-            break
-        key, value = line.split('=', 1)
-        req_vars.append((urllib.unquote(key), urllib.unquote(value)))
-
-    # Statements until the next blank line.
     statements = []
+    first_tb_line = ''
     for line in lines:
         line = line.strip()
         if line == '':
-            break
-        match = re.match(
-            r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line)
-        assert match is not None, (
-            "Unable to interpret oops line: %s" % line)
-        start, end, db_id, statement = match.groups()
-        if db_id is not None:
-            db_id = intern(db_id)  # This string is repeated lots.
-        statements.append(
-            (int(start), int(end), db_id, statement))
+            continue
+        else:
+            if is_req_var(line):
+                key, value = line.split('=', 1)
+                req_vars.append((urllib.unquote(key), urllib.unquote(value)))
+            elif is_statement(line):
+                match = re.match(
+                    r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line)
+                assert match is not None, (
+                    "Unable to interpret oops line: %s" % line)
+                start, end, db_id, statement = match.groups()
+                if db_id is not None:
+                    db_id = intern(db_id)  # This string is repeated lots.
+                statements.append(
+                    (int(start), int(end), db_id, statement))
+            elif is_traceback(line):
+                first_tb_line = line
+                break
 
     # The rest is traceback.
-    tb_text = ''.join(lines)
+    tb_text = ''.join([first_tb_line] + list(lines))
 
     result = dict(id=id, type=exc_type, value=exc_value, time=date,
             topic=topic, tb_text=tb_text, username=username, url=url,

=== modified file 'oops_datedir_repo/tests/test_serializer_rfc822.py'
--- oops_datedir_repo/tests/test_serializer_rfc822.py	2011-08-25 18:23:30 +0000
+++ oops_datedir_repo/tests/test_serializer_rfc822.py	2011-08-25 20:39:09 +0000
@@ -79,6 +79,37 @@
             report['db_statements'][1],
             (5, 10, 'store_b', 'SELECT 2'))
 
+    def test_read_blankline_req_vars(self):
+        """Test ErrorReport.read() for old logs with a blankline between
+        reqvars."""
+        fp = StringIO.StringIO(dedent("""\
+            Oops-Id: OOPS-A0001
+            Exception-Type: NotFound
+            Exception-Value: error message
+            Date: 2005-04-01T00:00:00+00:00
+            Topic: IFoo:+foo-template
+            User: Sample User
+            URL: http://localhost:9000/foo
+            Duration: 42
+
+            HTTP_USER_AGENT=Mozilla/5.0
+
+            HTTP_REFERER=http://localhost:9000/
+            name%3Dfoo=hello%0Aworld
+
+            00001-00005@store_a SELECT 1
+            00005-00010@store_b SELECT 2
+
+            traceback-text"""))
+        report = read(fp)
+        self.assertEqual(report['id'], 'OOPS-A0001')
+        self.assertEqual(report['req_vars'][0], ('HTTP_USER_AGENT',
+                                             'Mozilla/5.0'))
+        self.assertEqual(report['req_vars'][1], ('HTTP_REFERER',
+                                             'http://localhost:9000/'))
+        self.assertEqual(report['req_vars'][2], ('name=foo', 'hello\nworld'))
+        self.assertEqual(len(report['db_statements']), 2)
+
     def test_read_no_store_id(self):
         """Test ErrorReport.read() for old logs with no store_id."""
         fp = StringIO.StringIO(dedent("""\