[tor-commits] [stem/master] Raising TypeError when metrics header is unrecognized

[atagar at torproject.org]

commit a85e6f77ee1b4ba40e86bd148062bf8e479484e1
Author: Damian Johnson <atagar at torproject.org>
Date:   Sat Aug 18 16:12:35 2012 -0700

    Raising TypeError when metrics header is unrecognized
    
    When we get a metrics header for a format we don't recognize the
    _parse_metrics_file() function is supposed to throw a TypeError (and says it
    does in a comment). However, it didn't.
    
    This caused unrecognized metrics types to be parsed like a valid descriptor
    file containing zero descriptors. Fixing and adding a test for this use case.
    
    Also fixing a couple other minor bugs I ran into while fixing this:
    
    * Some tests had an assertTrue() rather than assertEqual(), causing the
      assertions to almost always evaluate to True.
    
    * The UnrecognizedType we threw for text files didn't have its mime type set.
---
 stem/descriptor/__init__.py                 |    2 +
 stem/descriptor/reader.py                   |    6 ++--
 test/integ/descriptor/data/new_metrics_type |    3 ++
 test/integ/descriptor/reader.py             |   29 ++++++++++++++++++++++----
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/stem/descriptor/__init__.py b/stem/descriptor/__init__.py
index 1372e17..8e2bdc7 100644
--- a/stem/descriptor/__init__.py
+++ b/stem/descriptor/__init__.py
@@ -110,6 +110,8 @@ def _parse_metrics_file(descriptor_type, major_version, minor_version, descripto
     yield stem.descriptor.networkstatus.parse_file(descriptor_file)
   elif descriptor_type == "network-status-microdesc-consensus-3" and major_version == 1:
     yield stem.descriptor.networkstatus.parse_file(descriptor_file, flavour = "microdesc")
+  else:
+    raise TypeError("Unrecognized metrics descriptor format. type: '%s', version: '%i.%i'" % (descriptor_type, major_version, minor_version))
 
 class Descriptor(object):
   """
diff --git a/stem/descriptor/reader.py b/stem/descriptor/reader.py
index 84ce695..eb2265b 100644
--- a/stem/descriptor/reader.py
+++ b/stem/descriptor/reader.py
@@ -423,14 +423,14 @@ class DescriptorReader(object):
     
     if target_type[0] in (None, 'text/plain'):
       # either '.txt' or an unknown type
-      self._handle_descriptor_file(target)
+      self._handle_descriptor_file(target, target_type)
     elif is_tar:
       # handles gzip, bz2, and decompressed tarballs among others
       self._handle_archive(target)
     else:
       self._notify_skip_listeners(target, UnrecognizedType(target_type))
   
-  def _handle_descriptor_file(self, target):
+  def _handle_descriptor_file(self, target, mime_type):
     try:
       with open(target) as target_file:
         for desc in stem.descriptor.parse_file(target, target_file):
@@ -438,7 +438,7 @@ class DescriptorReader(object):
           self._unreturned_descriptors.put(desc)
           self._iter_notice.set()
     except TypeError, exc:
-      self._notify_skip_listeners(target, UnrecognizedType(None))
+      self._notify_skip_listeners(target, UnrecognizedType(mime_type))
     except ValueError, exc:
       self._notify_skip_listeners(target, ParsingFailure(exc))
     except IOError, exc:
diff --git a/test/integ/descriptor/data/new_metrics_type b/test/integ/descriptor/data/new_metrics_type
new file mode 100644
index 0000000..95bd669
--- /dev/null
+++ b/test/integ/descriptor/data/new_metrics_type
@@ -0,0 +1,3 @@
+ at type non-existant-type 1.0
+valid metrics header, but a type we shouldn't recognize
+
diff --git a/test/integ/descriptor/reader.py b/test/integ/descriptor/reader.py
index 6ec538d..7b3a6da 100644
--- a/test/integ/descriptor/reader.py
+++ b/test/integ/descriptor/reader.py
@@ -350,12 +350,12 @@ class TestDescriptorReader(unittest.TestCase):
     
     with reader: list(reader) # iterates over all of the descriptors
     
-    self.assertTrue(2, len(skip_listener.results))
+    self.assertEqual(4, len(skip_listener.results))
     
     for skip_path, skip_exception in skip_listener.results:
       if skip_path.endswith(".swp"): continue # skip vim temp files
       
-      if not os.path.basename(skip_path) in ("riddle", "tiny.png", "vote"):
+      if not os.path.basename(skip_path) in ("riddle", "tiny.png", "vote", "new_metrics_type"):
         self.fail("Unexpected non-descriptor content: %s" % skip_path)
       
       self.assertTrue(isinstance(skip_exception, stem.descriptor.reader.UnrecognizedType))
@@ -406,7 +406,7 @@ class TestDescriptorReader(unittest.TestCase):
       reader.register_skip_listener(skip_listener.listener)
       with reader: list(reader) # iterates over all of the descriptors
       
-      self.assertTrue(1, len(skip_listener.results))
+      self.assertEqual(1, len(skip_listener.results))
       
       skipped_path, skip_exception = skip_listener.results[0]
       self.assertEqual(test_path, skipped_path)
@@ -438,7 +438,7 @@ class TestDescriptorReader(unittest.TestCase):
       reader.register_skip_listener(skip_listener.listener)
       with reader: list(reader) # iterates over all of the descriptors
       
-      self.assertTrue(1, len(skip_listener.results))
+      self.assertEqual(1, len(skip_listener.results))
       
       skipped_path, skip_exception = skip_listener.results[0]
       self.assertEqual(test_path, skipped_path)
@@ -460,9 +460,28 @@ class TestDescriptorReader(unittest.TestCase):
     reader.register_skip_listener(skip_listener.listener)
     with reader: list(reader) # iterates over all of the descriptors
     
-    self.assertTrue(1, len(skip_listener.results))
+    self.assertEqual(1, len(skip_listener.results))
     
     skipped_path, skip_exception = skip_listener.results[0]
     self.assertEqual(test_path, skipped_path)
     self.assertTrue(isinstance(skip_exception, stem.descriptor.reader.FileMissing))
+  
+  def test_unrecognized_metrics_type(self):
+    """
+    Parses a file that has a valid metrics header, but an unrecognized type.
+    """
+    
+    test_path = test.integ.descriptor.get_resource("new_metrics_type")
+    
+    skip_listener = SkipListener()
+    reader = stem.descriptor.reader.DescriptorReader(test_path)
+    reader.register_skip_listener(skip_listener.listener)
+    with reader: list(reader) # iterates over all of the descriptors
+    
+    self.assertEqual(1, len(skip_listener.results))
+    
+    skipped_path, skip_exception = skip_listener.results[0]
+    self.assertEqual(test_path, skipped_path)
+    self.assertTrue(isinstance(skip_exception, stem.descriptor.reader.UnrecognizedType))
+    self.assertEqual((None, None), skip_exception.mime_type)