Merge pull request #36 from python-hyper/idle-streams

Add support for parents that are idle.

HISTORY.rst

@@ -1,6 +1,14 @@
 Changelog
 =========
 
+1.2.1dev0
+---------
+
+**Bugfixes**
+
+- Allow insertion of streams that have parents in the idle or closed states.
+  This would previously raise a KeyError.
+
 1.2.0 (2016-08-04)
 ------------------
 

src/priority/priority.py

@@ -237,6 +237,20 @@ def __init__(self, maximum_streams=1000):
         self._streams = {0: self._root_stream}
         self._maximum_streams = maximum_streams
 
+    def _get_or_insert_parent(self, parent_stream_id):
+        """
+        When inserting or reprioritizing a stream it is possible to make it
+        dependent on a stream that is no longer in the tree. In this situation,
+        rather than bail out, we should insert the parent stream into the tree
+        with default priority and mark it as blocked.
+        """
+        try:
+            return self._streams[parent_stream_id]
+        except KeyError:
+            self.insert_stream(parent_stream_id)
+            self.block(parent_stream_id)
+            return self._streams[parent_stream_id]
+
     def _exclusive_insert(self, parent_stream, inserted_stream):
         """
         Insert ``inserted_stream`` beneath ``parent_stream``, obeying the
@@ -274,15 +288,15 @@ def insert_stream(self,
 
         if exclusive:
             assert depends_on is not None
-            parent_stream = self._streams[depends_on]
+            parent_stream = self._get_or_insert_parent(depends_on)
             self._exclusive_insert(parent_stream, stream)
             self._streams[stream_id] = stream
             return
 
         if not depends_on:
             depends_on = 0
 
-        parent = self._streams[depends_on]
+        parent = self._get_or_insert_parent(depends_on)
         parent.add_child(stream)
         self._streams[stream_id] = stream
 
@@ -333,8 +347,7 @@ def stream_cycle(new_parent, current):
         # own dependents. Then, we remove this stream from its current parent
         # and move it to its new parent, taking its children with it.
         if depends_on:
-            # TODO: What if we don't have the new parent?
-            new_parent = self._streams[depends_on]
+            new_parent = self._get_or_insert_parent(depends_on)
             cycle = stream_cycle(new_parent, current_stream)
         else:
             new_parent = self._streams[0]

test/test_priority.py

@@ -292,6 +292,70 @@ def test_priority_raises_good_errors_for_missing_streams(self):
         with pytest.raises(priority.MissingStreamError):
             p.remove_stream(3)
 
+    @pytest.mark.parametrize('exclusive', [True, False])
+    def test_priority_allows_inserting_stream_with_absent_parent(self,
+                                                                 exclusive):
+        """
+        Attemping to insert a stream that depends on a stream that is not in
+        the tree automatically inserts the parent with default priority.
+        """
+        p = priority.PriorityTree()
+        p.insert_stream(
+            stream_id=3, depends_on=1, exclusive=exclusive, weight=32
+        )
+
+        # Iterate 10 times to prove that the parent stream starts blocked.
+        first_ten_ids = [next(p) for _ in range(0, 10)]
+        assert first_ten_ids == [3] * 10
+
+        # Unblock the parent.
+        p.unblock(1)
+
+        # Iterate 10 times, expecting only the parent.
+        next_ten_ids = [next(p) for _ in range(0, 10)]
+        assert next_ten_ids == [1] * 10
+
+        # Insert a new stream into the tree with default priority.
+        p.insert_stream(stream_id=5)
+
+        # Iterate 10 more times. Expect the parent, and the new stream, in
+        # equal amounts.
+        next_ten_ids = [next(p) for _ in range(0, 10)]
+        assert next_ten_ids == [5, 1] * 5
+
+    @pytest.mark.parametrize('exclusive', [True, False])
+    def test_priority_reprioritizing_stream_with_absent_parent(self,
+                                                               exclusive):
+        """
+        Attemping to reprioritize a stream to depend on a stream that is not in
+        the tree automatically inserts the parent with default priority.
+        """
+        p = priority.PriorityTree()
+        p.insert_stream(stream_id=3)
+
+        p.reprioritize(
+            stream_id=3, depends_on=1, exclusive=exclusive, weight=32
+        )
+
+        # Iterate 10 times to prove that the parent stream starts blocked.
+        first_ten_ids = [next(p) for _ in range(0, 10)]
+        assert first_ten_ids == [3] * 10
+
+        # Unblock the parent.
+        p.unblock(1)
+
+        # Iterate 10 times, expecting only the parent.
+        next_ten_ids = [next(p) for _ in range(0, 10)]
+        assert next_ten_ids == [1] * 10
+
+        # Insert a new stream into the tree with default priority.
+        p.insert_stream(stream_id=5)
+
+        # Iterate 10 more times. Expect the parent, and the new stream, in
+        # equal amounts.
+        next_ten_ids = [next(p) for _ in range(0, 10)]
+        assert next_ten_ids == [5, 1] * 5
+
     @pytest.mark.parametrize('count', range(2, 10000, 100))
     def test_priority_refuses_to_allow_too_many_streams_in_tree(self, count):
         """