chromium-status: automatically linkify usernames/status messages

status.py

 # coding=utf-8
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Status management pages."""
 
 import datetime
 import json
 import re
 
 from google.appengine.api import memcache
 from google.appengine.ext import db
 
 from base_page import BasePage
 import utils
 
 
 ALLOWED_ORIGINS = [
     'https://gerrit-int.chromium.org',
     'https://gerrit.chromium.org',
 ]
 
 
+class Link(object):
+  """Simple object to hold text that might be linked"""

[M-A Ruel, 2013/11/25 16:18:14]

"""Holds text ..

No need to state a relative quality of the object in the docstring, here that it
is simple.

[vapier, 2013/11/25 17:51:26]

i agree it's simple, but i find scanning a one line docstring is a lot faster
than scanning the content

+
+  def __init__(self, text, target=None, is_email=False):

[M-A Ruel, 2013/11/25 16:18:14]

I'd prefer to not use default arguments.

[vapier, 2013/11/25 17:51:26]

any reason why ?  the intention is for email to be uncommon.

if you're really set on there not being defaults, then i might as well use the
python collections module and not create my own.

+    self.text = text
+    self.target = target
+    self.is_email = is_email
+
+  def __repr__(self):
+    return 'Link({%s->%s})' % (self.text, self.target)
+
+
+class LinkableText(object):
+  """Turn arbitrary text into a set of links"""

[M-A Ruel, 2013/11/25 16:18:14]

Turns
(imperative everywhere)

[vapier, 2013/11/25 17:51:26]

i dont see the point, but i also dont really care to debate

+
+  GERRIT_URLS = {
+      'chrome': 'https://chrome-internal-review.googlesource.com',

[M-A Ruel, 2013/11/25 16:18:14]

Very hardcoded. :/ Could these be entities?

[vapier, 2013/11/25 17:51:26]

i have no idea what "entities" are.  plus this file already hardcodes gerrit
URLs, and the idea is to support known link styles across multiple instances (so
that CL:12345 works both in chromium-status and chromiumos-status and so on).

+      'chromium': 'https://chromium-review.googlesource.com',
+  }
+
+  WATERFALL_URLS = {
+      'chromeos': 'http://chromegw/i/chromeos',
+      'chromiumos': 'http://build.chromium.org/p/chromiumos',
+  }
+
+  # Automatically linkify convert known strings for the user.
+  _CONVERTS = [
+      # Convert CrOS bug links.  Support the forms:
+      # http://crbug.com/1234
+      # http://crosbug.com/1234
+      # crbug/1234
+      # crosbug/p/1234
+      (re.compile(
+           #   1   2      3      4        5       6 7
+           r'\b((http://)?((crbug|crosbug)(\.com)?(/(p/)?[0-9]+)))\b',
+           flags=re.I),
+       r'http://\4.com\6', r'\1', False),
+
+      # Convert e-mail addresses.
+      (re.compile(r'(([-+.a-z0-9_!#$%&*/=?^_`{|}~]+)@[-a-z0-9.]+\.[a-z0-9]+)\b',
+                  flags=re.I),
+       r'\1', r'\2', True),
+
+      # Convert SHA1's to gerrit links.  Assume all external since
+      # there is no sane way to detect it's an internal CL.
+      (re.compile(r'\b([0-9a-f]{40})\b', flags=re.I),
+       r'%s/#q,\1,n,z' % GERRIT_URLS['chromium'], r'\1', False),
+
+      # Convert public gerrit CL numbers.
+      (re.compile(r'\b(CL:([0-9]+))\b', flags=re.I),
+       r'%s/\2' % GERRIT_URLS['chromium'], r'\1', False),
+      # Convert internal gerrit CL numbers.
+      (re.compile(r'\b(CL:\*([0-9]+))\b', flags=re.I),
+       r'%s/\2' % GERRIT_URLS['chrome'], r'\1', False),
+  ]
+
+  @classmethod
+  def bootstrap(cls, _app_name):
+    """Add conversions specific to |app_name| instance"""
+    cls._CONVERTS += [
+      # Do this for everyone since "cbuildbot" is unique to CrOS.
+      # Match the string:
+      #   Automatic: "cbuildbot" on "x86-generic ASAN" from.
+      (re.compile(r'("cbuildbot" on "([^"]+ canary)")',

[M-A Ruel, 2013/11/25 16:18:14]

These should be stated as static lists. There isn't much dynamic in there,
unless you properly create entities to store the instance-specific rules, which
I prefer.

You can work around re.I by using r'(?i)' as a prefix to your regexp.

[vapier, 2013/11/25 17:51:26]

this was needed in a previous CL where i used a function callback.  i no longer
use that, so i no longer need to bootstrap these rules on the fly.

the re.compile/re.flags stuff aren't a problem to do at class decl time (as i'm
already doing that above).

+                  flags=re.I),
+       r'%s/builders/\2' % cls.WATERFALL_URLS['chromeos'], r'\1', False),
+      (re.compile(r'("cbuildbot" on "([^"]+)")',
+                  flags=re.I),
+       r'%s/builders/\2' % cls.WATERFALL_URLS['chromiumos'], r'\1', False),
+    ]
+
+  @classmethod
+  def parse(cls, text):
+    """Create a list of Link objects based on |text|"""
+    if not text:
+      return []
+    for prog, target, pretty_text, is_email in cls._CONVERTS:
+      m = prog.search(text)
+      if m:
+        link = Link(m.expand(pretty_text),
+                    target=m.expand(target),
+                    is_email=is_email)
+        left_links = cls.parse(text[:m.start()].rstrip())
+        right_links = cls.parse(text[m.end():].lstrip())
+        return left_links + [link] + right_links
+    return [Link(text)]
+
+  def __init__(self, text):
+    self.raw_text = text
+    self.links = self.parse(text.strip())
+
+  def __str__(self):
+    return self.raw_text
+
+
 class Status(db.Model):
   """Description for the status table."""
   # The username who added this status.
   username = db.StringProperty(required=True)
   # The date when the status got added.
   date = db.DateTimeProperty(auto_now_add=True)
   # The message. It can contain html code.
   message = db.StringProperty(required=True)
 
+  def __init__(self, *args, **kwargs):
+    # Normalize newlines otherwise the DB store barfs.

[M-A Ruel, 2013/11/25 16:18:14]

I'm fine with adding multiline=True instead. Actually,  I prefer that. The
constructor may not always be used.

[vapier, 2013/11/25 17:51:26]

i don't think we want people to insert newlines into their messages.  i'm not
sure the various parts of the system (or the clients) are ready to handle it,
and it doesn't make sense to even bother supporting it.

+    kwargs['message'] = kwargs.get('message', '').replace('\n', '')
+
+    super(Status, self).__init__(*args, **kwargs)
+    self.username_links = LinkableText(self.username)
+    self.message_links = LinkableText(self.message)
+
   @property
   def general_state(self):
     """Returns a string representing the state that the status message
     describes.
     """
     message = self.message
     closed = re.search('close', message, re.IGNORECASE)
     if closed and re.search('maint', message, re.IGNORECASE):
       return 'maintenance'
     if re.search('throt', message, re.IGNORECASE):
@@ skipping 195 matching lines @@
 
   # NOTE: This is require_login in order to ensure that authentication doesn't
   # happen while changing the tree status.
   @utils.requires_login
   @utils.requires_read_access
   def get(self):
     return self._handle()
 
   def _handle(self, error_message='', last_message=''):
     """Sets the information to be displayed on the main page."""
+    LinkableText.bootstrap(self.APP_NAME)

[M-A Ruel, 2013/11/25 16:18:14]

This is a bad idea to do this there. It creates a memory leak as
LinkableText._CONVERT grows on each root page rendering. There's a bootstrap
function at the end to manage on-startup issues like that.

[vapier, 2013/11/25 17:51:26]

i'm not terribly familiar with the app engine model.  i could make it so the
func itself handles being called more than once.

i also had it here because i thought the app_name stuff was initialized later,
and to use the api as available.  but i guess that is setup in the basepage's
bootstrap phase, and is set on the class level, so i can just use that directly.

+
     try:
       limit = min(max(int(self.request.get('limit')), 1), 1000)
     except ValueError:
       limit = 25
     status = get_last_statuses(limit)
     current_status = get_status()
     if not last_message:
       last_message = current_status.message
 
     template_values = self.InitializeTemplate(self.APP_NAME + ' Tree Status')
@@ skipping 30 matching lines @@
       return self._handle(error_message, last_message)
     else:
       put_status(Status(message=new_message, username=self.user.email()))
       self.redirect("/")
 
 
 def bootstrap():
   # Guarantee that at least one instance exists.
   if db.GqlQuery('SELECT __key__ FROM Status').get() is None:
     Status(username='none', message='welcome to status').put()