[Predator] Add ``Project`` class and ``ClassifySuspect`` method to project and component classifier

appengine/findit/crash/component_classifier.py

Index: appengine/findit/crash/component_classifier.py
diff --git a/appengine/findit/crash/component_classifier.py b/appengine/findit/crash/component_classifier.py
index 5e6c8287cdbf2260a2c4bad489a6c097d63f70f6..7dbca4c59a2fab993716778e9e374374ed6179d6 100644
--- a/appengine/findit/crash/component_classifier.py
+++ b/appengine/findit/crash/component_classifier.py
@@ -4,6 +4,7 @@
 
 from collections import namedtuple
 from collections import defaultdict
+import functools
 import logging
 import re
 
@@ -18,89 +19,59 @@ class ComponentClassifier(object):
   For example: ['Blink>DOM', 'Blink>HTML'].
   """
 
-  def __init__(self, components, top_n):
+  def __init__(self, components, top_n_frames):
     """Build a classifier for components.
 
     Args:
       components (list of crash.component.Component): the components to
         check for.
-      top_n (int): how many frames of the callstack to look at"""
-    super(ComponentClassifier, self).__init__()
-    if not components:
-      logging.warning('Empty configuration for component classifier.')
-      components = [] # Ensure self.components is not None
-    self.components = components
-    self.top_n = top_n
-
-  def GetClassFromStackFrame(self, frame):
-    """Determine which component is responsible for this frame."""
-    for component in self.components:
-      if component.MatchesStackFrame(frame):
-        return component.component_name
-
-    return ''
-
-  # TODO(wrengr): refactor this into a method on Suspect which returns
-  # the cannonical frame (and documents why it's the one we return).
-  def GetClassFromSuspect(self, suspect):
-    """Determine which component is responsible for this suspect.
-
-    Note that Findit assumes files that the culprit suspect touched come from
-    the same component.
+      top_n_frames (int): how many frames of the callstack to look at
     """
-    if suspect.file_to_stack_infos:
-      # file_to_stack_infos is a dict mapping file_path to stack_infos,
-      # where stack_infos is a list of (frame, callstack_priority)
-      # pairs. So ``.values()`` returns a list of the stack_infos in an
-      # arbitrary order; the first ``[0]`` grabs the "first" stack_infos;
-      # the second ``[0]`` grabs the first pair from the list; and
-      # the third ``[0]`` grabs the ``frame`` from the pair.
-      # TODO(wrengr): why is that the right frame to look at?
-      frame = suspect.file_to_stack_infos.values()[0][0][0]
-      return self.GetClassFromStackFrame(frame)
-
-    return ''
+    super(ComponentClassifier, self).__init__()
+    self.components = [] if components is None else components
+    self.top_n_frames = top_n_frames
 
   # TODO(http://crbug.com/657177): return the Component objects
   # themselves, rather than strings naming them.
-  def Classify(self, suspects, crash_stack):
+  def ClassifyCallStack(self, crash_stack, top_n=2):
     """Classifies component of a crash.
 
     Args:
-      suspects (list of Suspect): Culprit suspects.
       crash_stack (CallStack): The callstack that caused the crash.
 

[chanli, 2017/01/26 21:31:06]

Add docstring for top_n

[Sharu Jiang, 2017/01/27 03:20:42]

Done.

     Returns:
       List of top 2 components.
     """
-    # If ``suspects`` are available, we use the components from there since
-    # they're more reliable than the ones from the ``crash_stack``.
-    if suspects:
-      classes = map(self.GetClassFromSuspect, suspects[:self.top_n])
-    else:
-      classes = map(self.GetClassFromStackFrame,
-          crash_stack.frames[:self.top_n])
-
-    return RankByOccurrence(classes, 2)
-
-  # TODO(ymzhang): use component of new path as default. RENAME might
-  # need to return two (old path new path may have different components)
-  def GetClassFromFileChangeInfo(self, file_change_info):
-    """Determine which component is responsible for a touched file."""
-    if not file_change_info:
+    def _GetClassFromStackFrame(frame):
+      """Determine which component is responsible for this frame."""
+      for component in self.components:
+        if component.MatchesStackFrame(frame):
+          return component.component_name
+
       return None
 
-    for component in self.components:
-      if (file_change_info.change_type == ChangeType.DELETE):
-        if component.MatchesFile(file_change_info.old_path):
-          return component.component_name
-      else:
-        if component.MatchesFile(file_change_info.new_path):
-          return component.component_name
+    classes = map(_GetClassFromStackFrame,
+                  crash_stack.frames[:self.top_n_frames])
+    return RankByOccurrence(classes, top_n) or None
 
-    return ''
+  # TODO(http://crbug.com/657177): return the Component objects
+  # themselves, rather than strings naming them.
+  def ClassifySuspects(self, suspects, top_n=2):
+    """Classifies component of a list of suspects.
 
-  def ClassifyChangeLog(self, change_log, top_n=2):
+    Args:
+      suspects (list of Suspect): Culprit suspects.
+
+    Returns:
+      List of top 2 components.
+    """
+    classes = []
+    for suspect in suspects:
+      classes.extend(self.ClassifySuspect(suspect))
+
+    return RankByOccurrence(classes, top_n) or None
+
+  def ClassifySuspect(self, suspect, top_n=2):
     """ Classifies components of a change log.

[chanli, 2017/01/26 21:31:06]

Nit: update docstring and args

[Sharu Jiang, 2017/01/27 03:20:42]

Done.

 
      Args:
@@ -110,8 +81,20 @@ class ComponentClassifier(object):
      Returns:
        List of components
      """
-    if not change_log:
+    if not suspect or not suspect.changelog:
+      return None
+
+    # TODO(ymzhang): use component of new path as default. RENAME might
+    # need to return two (old path new path may have different components)
+    def _GetClassFromTouchedFile(dep_path, touched_file):

[chanli, 2017/01/26 21:31:06]

How does GetClassFromTouchedFile get touched_file?

[Sharu Jiang, 2017/01/27 03:20:42]

it is the parameter of the function.

+      """Determine which component is responsible for a touched file."""
+      for component in self.components:
+        if component.MatchesTouchedFile(dep_path, touched_file):
+          return component.component_name
+
       return None
 
-    classes = map(self.GetClassFromFileChangeInfo, change_log.touched_files)
-    return RankByOccurrence(classes, top_n, rank_function=lambda x:-len(x))
+    get_class = functools.partial(_GetClassFromTouchedFile, suspect.dep_path)
+    classes = map(get_class, suspect.changelog.touched_files)
+    return RankByOccurrence(classes, top_n,
+                            rank_function=lambda x:-len(x)) or None