Merge pull request #3202 from madt1m/view-cleanup

View Cleanup - Initial steps

examples/complex/dup_and_replay.py

@@ -9,6 +9,6 @@ def request(flow):
     # Only interactive tools have a view. If we have one, add a duplicate entry
     # for our flow.
     if "view" in ctx.master.addons:
-        ctx.master.commands.call("view.add", [flow])
+        ctx.master.commands.call("view.flows.add", [flow])
     flow.request.path = "/changed"
     ctx.master.commands.call("replay.client", [flow])

mitmproxy/addons/view.py

@@ -166,12 +166,6 @@ class View(collections.Sequence):
     def store_count(self):
         return len(self._store)
 
-    def inbounds(self, index: int) -> bool:
-        """
-            Is this 0 <= index < len(self)
-        """
-        return 0 <= index < len(self)
-
     def _rev(self, idx: int) -> int:
         """
             Reverses an index, if needed
@@ -219,7 +213,26 @@ class View(collections.Sequence):
                 self._base_add(i)
         self.sig_view_refresh.send(self)
 
-    # API
+    """ View API """
+
+    # Focus
+    @command.command("view.focus.go")
+    def go(self, dst: int) -> None:
+        """
+            Go to a specified offset. Positive offests are from the beginning of
+            the view, negative from the end of the view, so that 0 is the first
+            flow, -1 is the last flow.
+        """
+        if len(self) == 0:
+            return
+        if dst < 0:
+            dst = len(self) + dst
+        if dst < 0:
+            dst = 0
+        if dst > len(self) - 1:
+            dst = len(self) - 1
+        self.focus.flow = self[dst]
+
     @command.command("view.focus.next")
     def focus_next(self) -> None:
         """
@@ -238,6 +251,7 @@ class View(collections.Sequence):
         if self.inbounds(idx):
             self.focus.flow = self[idx]
 
+    # Order
     @command.command("view.order.options")
     def order_options(self) -> typing.Sequence[str]:
         """
@@ -245,34 +259,58 @@ class View(collections.Sequence):
         """
         return list(sorted(self.orders.keys()))
 
-    @command.command("view.marked.toggle")
+    @command.command("view.order.reverse")
-    def toggle_marked(self) -> None:
+    def set_reversed(self, value: bool) -> None:
-        """
-            Toggle whether to show marked views only.
-        """
-        self.show_marked = not self.show_marked
-        self._refilter()
-
-    def set_reversed(self, value: bool):
         self.order_reversed = value
         self.sig_view_refresh.send(self)
 
-    def set_order(self, order_key: typing.Callable):
+    @command.command("view.order.set")
+    def set_order(self, order: str) -> None:
         """
             Sets the current view order.
         """
+        if order not in self.orders:
+            raise exceptions.CommandError(
+                "Unknown flow order: %s" % order
+            )
+        order_key = self.orders[order]
         self.order_key = order_key
         newview = sortedcontainers.SortedListWithKey(key=order_key)
         newview.update(self._view)
         self._view = newview
 
-    def set_filter(self, flt: typing.Optional[flowfilter.TFilter]):
+    @command.command("view.order")
+    def get_order(self) -> str:
+        """
+        Returns the current view order.
+        """
+        order = ""
+        for k in self.orders.keys():
+            if self.order_key == self.orders[k]:
+                order = k
+        return order
+
+    # Filter
+    @command.command("view.filter.set")
+    def set_filter_cmd(self, f: str) -> None:
         """
             Sets the current view filter.
         """
+        filt = None
+        if f:
+            filt = flowfilter.parse(f)
+            if not filt:
+                raise exceptions.CommandError(
+                    "Invalid interception filter: %s" % f
+                )
+        self.set_filter(filt)
+
+    def set_filter(self, flt: typing.Optional[flowfilter.TFilter]):
         self.filter = flt or matchall
         self._refilter()
 
+    # View Updates
+    @command.command("view.clear")
     def clear(self) -> None:
         """
             Clears both the store and view.
@@ -282,7 +320,8 @@ class View(collections.Sequence):
         self.sig_view_refresh.send(self)
         self.sig_store_refresh.send(self)
 
-    def clear_not_marked(self):
+    @command.command("view.clear_unmarked")
+    def clear_not_marked(self) -> None:
         """
             Clears only the unmarked flows.
         """
@@ -293,36 +332,15 @@ class View(collections.Sequence):
         self._refilter()
         self.sig_store_refresh.send(self)
 
-    @command.command("view.marked.toggle")
+    # View Settings
-    def add(self, flows: typing.Sequence[mitmproxy.flow.Flow]) -> None:
+    @command.command("view.settings.getval")
-        """
-            Adds a flow to the state. If the flow already exists, it is
-            ignored.
-        """
-        for f in flows:
-            if f.id not in self._store:
-                self._store[f.id] = f
-                if self.filter(f):
-                    self._base_add(f)
-                    if self.focus_follow:
-                        self.focus.flow = f
-                    self.sig_view_add.send(self, flow=f)
-
-    def get_by_id(self, flow_id: str) -> typing.Optional[mitmproxy.flow.Flow]:
-        """
-        Get flow with the given id from the store.
-        Returns None if the flow is not found.
-        """
-        return self._store.get(flow_id)
-
-    @command.command("view.getval")
     def getvalue(self, f: mitmproxy.flow.Flow, key: str, default: str) -> str:
         """
             Get a value from the settings store for the specified flow.
         """
         return self.settings[f].get(key, default)
 
-    @command.command("view.setval.toggle")
+    @command.command("view.settings.setval.toggle")
     def setvalue_toggle(
         self,
         flows: typing.Sequence[mitmproxy.flow.Flow],
@@ -339,7 +357,7 @@ class View(collections.Sequence):
             updated.append(f)
         ctx.master.addons.trigger("update", updated)
 
-    @command.command("view.setval")
+    @command.command("view.settings.setval")
     def setvalue(
         self,
         flows: typing.Sequence[mitmproxy.flow.Flow],
@@ -354,41 +372,8 @@ class View(collections.Sequence):
             updated.append(f)
         ctx.master.addons.trigger("update", updated)
 
-    @command.command("view.load")
+    # Flows
-    def load_file(self, path: mitmproxy.types.Path) -> None:
+    @command.command("view.flows.duplicate")
-        """
-            Load flows into the view, without processing them with addons.
-        """
-        try:
-            with open(path, "rb") as f:
-                for i in io.FlowReader(f).stream():
-                    # Do this to get a new ID, so we can load the same file N times and
-                    # get new flows each time. It would be more efficient to just have a
-                    # .newid() method or something.
-                    self.add([i.copy()])
-        except IOError as e:
-            ctx.log.error(e.strerror)
-        except exceptions.FlowReadException as e:
-            ctx.log.error(str(e))
-
-    @command.command("view.go")
-    def go(self, dst: int) -> None:
-        """
-            Go to a specified offset. Positive offests are from the beginning of
-            the view, negative from the end of the view, so that 0 is the first
-            flow, -1 is the last flow.
-        """
-        if len(self) == 0:
-            return
-        if dst < 0:
-            dst = len(self) + dst
-        if dst < 0:
-            dst = 0
-        if dst > len(self) - 1:
-            dst = len(self) - 1
-        self.focus.flow = self[dst]
-
-    @command.command("view.duplicate")
     def duplicate(self, flows: typing.Sequence[mitmproxy.flow.Flow]) -> None:
         """
             Duplicates the specified flows, and sets the focus to the first
@@ -400,7 +385,7 @@ class View(collections.Sequence):
             self.focus.flow = dups[0]
             ctx.log.alert("Duplicated %s flows" % len(dups))
 
-    @command.command("view.remove")
+    @command.command("view.flows.remove")
     def remove(self, flows: typing.Sequence[mitmproxy.flow.Flow]) -> None:
         """
             Removes the flow from the underlying store and the view.
@@ -420,7 +405,7 @@ class View(collections.Sequence):
         if len(flows) > 1:
             ctx.log.alert("Removed %s flows" % len(flows))
 
-    @command.command("view.resolve")
+    @command.command("view.flows.resolve")
     def resolve(self, spec: str) -> typing.Sequence[mitmproxy.flow.Flow]:
         """
             Resolve a flow list specification to an actual list of flows.
@@ -443,7 +428,7 @@ class View(collections.Sequence):
                 raise exceptions.CommandError("Invalid flow filter: %s" % spec)
             return [i for i in self._store.values() if filt(i)]
 
-    @command.command("view.create")
+    @command.command("view.flows.create")
     def create(self, method: str, url: str) -> None:
         try:
             req = http.HTTPRequest.make(method.upper(), url)
@@ -456,6 +441,74 @@ class View(collections.Sequence):
         f.request.headers["Host"] = req.host
         self.add([f])
 
+    @command.command("view.flows.load")
+    def load_file(self, path: mitmproxy.types.Path) -> None:
+        """
+            Load flows into the view, without processing them with addons.
+        """
+        try:
+            with open(path, "rb") as f:
+                for i in io.FlowReader(f).stream():
+                    # Do this to get a new ID, so we can load the same file N times and
+                    # get new flows each time. It would be more efficient to just have a
+                    # .newid() method or something.
+                    self.add([i.copy()])
+        except IOError as e:
+            ctx.log.error(e.strerror)
+        except exceptions.FlowReadException as e:
+            ctx.log.error(str(e))
+
+    def add(self, flows: typing.Sequence[mitmproxy.flow.Flow]) -> None:
+        """
+            Adds a flow to the state. If the flow already exists, it is
+            ignored.
+        """
+        for f in flows:
+            if f.id not in self._store:
+                self._store[f.id] = f
+                if self.filter(f):
+                    self._base_add(f)
+                    if self.focus_follow:
+                        self.focus.flow = f
+                    self.sig_view_add.send(self, flow=f)
+
+    def get_by_id(self, flow_id: str) -> typing.Optional[mitmproxy.flow.Flow]:
+        """
+            Get flow with the given id from the store.
+            Returns None if the flow is not found.
+        """
+        return self._store.get(flow_id)
+
+    # View Properties
+    @command.command("view.properties.length")
+    def get_length(self) -> int:
+        """
+            Returns view length.
+        """
+        return len(self)
+
+    @command.command("view.properties.marked")
+    def get_marked(self) -> bool:
+        """
+            Returns true if view is in marked mode.
+        """
+        return self.show_marked
+
+    @command.command("view.properties.marked.toggle")
+    def toggle_marked(self) -> None:
+        """
+            Toggle whether to show marked views only.
+        """
+        self.show_marked = not self.show_marked
+        self._refilter()
+
+    @command.command("view.properties.inbounds")
+    def inbounds(self, index: int) -> bool:
+        """
+            Is this 0 <= index < len(self)?
+        """
+        return 0 <= index < len(self)
+
     # Event handlers
     def configure(self, updated):
         if "view_filter" in updated:
@@ -472,7 +525,7 @@ class View(collections.Sequence):
                 raise exceptions.OptionsError(
                     "Unknown flow order: %s" % ctx.options.view_order
                 )
-            self.set_order(self.orders[ctx.options.view_order])
+            self.set_order(ctx.options.view_order)
         if "view_order_reversed" in updated:
             self.set_reversed(ctx.options.view_order_reversed)
         if "console_focus_follow" in updated:

mitmproxy/tools/console/consoleaddons.py

@@ -515,7 +515,7 @@ class ConsoleAddon:
 
         try:
             self.master.commands.call_strings(
-                "view.setval",
+                "view.settings.setval",
                 ["@focus", "flowview_mode_%s" % idx, mode]
             )
         except exceptions.CommandError as e:
@@ -538,7 +538,7 @@ class ConsoleAddon:
             raise exceptions.CommandError("Not viewing a flow.")
         idx = fv.body.tab_offset
         return self.master.commands.call_strings(
-            "view.getval",
+            "view.settings.getval",
             [
                 "@focus",
                 "flowview_mode_%s" % idx,

mitmproxy/tools/console/defaultkeys.py

@@ -36,8 +36,8 @@ def map(km):
         ["flowlist", "flowview"],
         "Save response body to file"
     )
-    km.add("d", "view.remove @focus", ["flowlist", "flowview"], "Delete flow from view")
+    km.add("d", "view.flows.remove @focus", ["flowlist", "flowview"], "Delete flow from view")
-    km.add("D", "view.duplicate @focus", ["flowlist", "flowview"], "Duplicate flow")
+    km.add("D", "view.flows.duplicate @focus", ["flowlist", "flowview"], "Duplicate flow")
     km.add(
         "e",
         """
@@ -57,7 +57,7 @@ def map(km):
     )
     km.add("L", "console.command view.load ", ["flowlist"], "Load flows from file")
     km.add("m", "flow.mark.toggle @focus", ["flowlist"], "Toggle mark on this flow")
-    km.add("M", "view.marked.toggle", ["flowlist"], "Toggle viewing marked flows")
+    km.add("M", "view.properties.marked.toggle", ["flowlist"], "Toggle viewing marked flows")
     km.add(
         "n",
         "console.command view.create get https://example.com/",
@@ -80,8 +80,8 @@ def map(km):
     km.add("w", "console.command save.file @shown ", ["flowlist"], "Save listed flows to file")
     km.add("V", "flow.revert @focus", ["flowlist", "flowview"], "Revert changes to this flow")
     km.add("X", "flow.kill @focus", ["flowlist"], "Kill this flow")
-    km.add("z", "view.remove @all", ["flowlist"], "Clear flow list")
+    km.add("z", "view.flows.remove @all", ["flowlist"], "Clear flow list")
-    km.add("Z", "view.remove @hidden", ["flowlist"], "Purge all flows not showing")
+    km.add("Z", "view.flows.remove @hidden", ["flowlist"], "Purge all flows not showing")
     km.add(
         "|",
         "console.command script.run @focus ",
@@ -100,7 +100,7 @@ def map(km):
     )
     km.add(
         "f",
-        "view.setval.toggle @focus fullcontents",
+        "view.settings.setval.toggle @focus fullcontents",
         ["flowview"],
         "Toggle viewing full contents on this flow",
     )

mitmproxy/tools/console/flowlist.py

@@ -42,7 +42,7 @@ class FlowListWalker(urwid.ListWalker):
     def positions(self, reverse=False):
         # The stub implementation of positions can go once this issue is resolved:
         # https://github.com/urwid/urwid/issues/294
-        ret = range(len(self.master.view))
+        ret = range(self.master.commands.execute("view.properties.length"))
         if reverse:
             return reversed(ret)
         return ret
@@ -57,19 +57,19 @@ class FlowListWalker(urwid.ListWalker):
         return f, self.master.view.focus.index
 
     def set_focus(self, index):
-        if self.master.view.inbounds(index):
+        if self.master.commands.execute("view.properties.inbounds %d" % index):
             self.master.view.focus.index = index
 
     def get_next(self, pos):
         pos = pos + 1
-        if not self.master.view.inbounds(pos):
+        if not self.master.commands.execute("view.properties.inbounds %d" % pos):
             return None, None
         f = FlowItem(self.master, self.master.view[pos])
         return f, pos
 
     def get_prev(self, pos):
         pos = pos - 1
-        if not self.master.view.inbounds(pos):
+        if not self.master.commands.execute("view.properties.inbounds %d" % pos):
             return None, None
         f = FlowItem(self.master, self.master.view[pos])
         return f, pos
@@ -87,9 +87,9 @@ class FlowListBox(urwid.ListBox, layoutwidget.LayoutWidget):
 
     def keypress(self, size, key):
         if key == "m_start":
-            self.master.commands.execute("view.go 0")
+            self.master.commands.execute("view.focus.go 0")
         elif key == "m_end":
-            self.master.commands.execute("view.go -1")
+            self.master.commands.execute("view.focus.go -1")
         elif key == "m_select":
             self.master.commands.execute("console.view.flow @focus")
         return urwid.ListBox.keypress(self, size, key)

mitmproxy/tools/console/flowview.py

@@ -98,7 +98,7 @@ class FlowDetails(tabs.Tabs):
             msg, body = "", [urwid.Text([("error", "[content missing]")])]
             return msg, body
         else:
-            full = self.master.commands.execute("view.getval @focus fullcontents false")
+            full = self.master.commands.execute("view.settings.getval @focus fullcontents false")
             if full == "true":
                 limit = sys.maxsize
             else:

mitmproxy/tools/console/statusbar.py

@@ -271,7 +271,7 @@ class StatusBar(urwid.WidgetWrap):
         return r
 
     def redraw(self):
-        fc = len(self.master.view)
+        fc = self.master.commands.execute("view.properties.length")
         if self.master.view.focus.flow is None:
             offset = 0
         else:
@@ -283,7 +283,7 @@ class StatusBar(urwid.WidgetWrap):
             arrow = common.SYMBOL_DOWN
 
         marked = ""
-        if self.master.view.show_marked:
+        if self.master.commands.execute("view.properties.marked"):
             marked = "M"
 
         t = [

mitmproxy/types.py

@@ -337,7 +337,7 @@ class _FlowType(_BaseFlowType):
 
     def parse(self, manager: _CommandBase, t: type, s: str) -> flow.Flow:
         try:
-            flows = manager.call_strings("view.resolve", [s])
+            flows = manager.call_strings("view.flows.resolve", [s])
         except exceptions.CommandError as e:
             raise exceptions.TypeError from e
         if len(flows) != 1:
@@ -356,7 +356,7 @@ class _FlowsType(_BaseFlowType):
 
     def parse(self, manager: _CommandBase, t: type, s: str) -> typing.Sequence[flow.Flow]:
         try:
-            return manager.call_strings("view.resolve", [s])
+            return manager.call_strings("view.flows.resolve", [s])
         except exceptions.CommandError as e:
             raise exceptions.TypeError from e
 

test/mitmproxy/addons/test_view.py

@@ -107,13 +107,12 @@ def test_simple():
 
 def test_filter():
     v = view.View()
-    f = flowfilter.parse("~m get")
     v.request(tft(method="get"))
     v.request(tft(method="put"))
     v.request(tft(method="get"))
     v.request(tft(method="put"))
     assert(len(v)) == 4
-    v.set_filter(f)
+    v.set_filter_cmd("~m get")
     assert [i.request.method for i in v] == ["GET", "GET"]
     assert len(v._store) == 4
     v.set_filter(None)
@@ -124,6 +123,9 @@ def test_filter():
     v.toggle_marked()
     assert len(v) == 4
 
+    with pytest.raises(exceptions.CommandError):
+        v.set_filter_cmd("~notafilter regex")
+
     v[1].marked = True
     v.toggle_marked()
     assert len(v) == 1
@@ -303,23 +305,26 @@ def test_setgetval():
 
 def test_order():
     v = view.View()
-    with taddons.context(v) as tctx:
+    v.request(tft(method="get", start=1))
-        v.request(tft(method="get", start=1))
+    v.request(tft(method="put", start=2))
-        v.request(tft(method="put", start=2))
+    v.request(tft(method="get", start=3))
-        v.request(tft(method="get", start=3))
+    v.request(tft(method="put", start=4))
-        v.request(tft(method="put", start=4))
+    assert [i.request.timestamp_start for i in v] == [1, 2, 3, 4]
-        assert [i.request.timestamp_start for i in v] == [1, 2, 3, 4]
 
-        tctx.configure(v, view_order="method")
+    v.set_order("method")
-        assert [i.request.method for i in v] == ["GET", "GET", "PUT", "PUT"]
+    assert v.get_order() == "method"
-        v.set_reversed(True)
+    assert [i.request.method for i in v] == ["GET", "GET", "PUT", "PUT"]
-        assert [i.request.method for i in v] == ["PUT", "PUT", "GET", "GET"]
+    v.set_reversed(True)
+    assert [i.request.method for i in v] == ["PUT", "PUT", "GET", "GET"]
 
-        tctx.configure(v, view_order="time")
+    v.set_order("time")
-        assert [i.request.timestamp_start for i in v] == [4, 3, 2, 1]
+    assert v.get_order() == "time"
+    assert [i.request.timestamp_start for i in v] == [4, 3, 2, 1]
 
-        v.set_reversed(False)
+    v.set_reversed(False)
-        assert [i.request.timestamp_start for i in v] == [1, 2, 3, 4]
+    assert [i.request.timestamp_start for i in v] == [1, 2, 3, 4]
+    with pytest.raises(exceptions.CommandError):
+        v.set_order("not_an_order")
 
 
 def test_reversed():
@@ -551,6 +556,17 @@ def test_settings():
     assert not v.settings.keys()
 
 
+def test_properties():
+    v = view.View()
+    f = tft()
+    v.request(f)
+    assert v.get_length() == 1
+    assert not v.get_marked()
+    v.toggle_marked()
+    assert v.get_length() == 0
+    assert v.get_marked()
+
+
 def test_configure():
     v = view.View()
     with taddons.context(v) as tctx:

test/mitmproxy/test_command.py

@@ -313,7 +313,7 @@ def test_typename():
 
 
 class DummyConsole:
-    @command.command("view.resolve")
+    @command.command("view.flows.resolve")
     def resolve(self, spec: str) -> typing.Sequence[flow.Flow]:
         n = int(spec)
         return [tflow.tflow(resp=True)] * n

test/mitmproxy/test_types.py

@@ -146,7 +146,7 @@ def test_strseq():
 
 
 class DummyConsole:
-    @command.command("view.resolve")
+    @command.command("view.flows.resolve")
     def resolve(self, spec: str) -> typing.Sequence[flow.Flow]:
         if spec == "err":
             raise mitmproxy.exceptions.CommandError()