improve ConnectionStatus and welcome-page display

* replace "last_details" with "non_connected_statuses" dict
* rename "last_connection_summary" to just "summary"
* for connected servers, show other hints in a tooltip
* for not-yet-connected servers, show all hints in a list
* build the list (in STAN) on the server side, not using IContainer

src/allmydata/interfaces.py

@@ -2857,7 +2857,7 @@ class IConnectionStatus(Interface):
         negotiation was successful. Otherwise it is None.
         """)
 
-    last_connection_summary = Attribute(
+    summary = Attribute(
         """
         A string with a brief summary of the current status, suitable for
         display on an informational page. The more complete text from
@@ -2865,21 +2865,6 @@ class IConnectionStatus(Interface):
         popup.
         """)
 
-    last_connection_description = Attribute(
-        """
-        A string with a description of the results of the most recent
-        connection attempt. For Foolscap connections, this indicates the
-        winning hint and the connection handler which used it, e.g.
-        'tcp:HOST:PORT via tcp' or 'tor:HOST.onion:PORT via tor':
-
-        * 'Connection successful: HINT via HANDLER (other hints: ..)'
-        * 'Connection failed: HINT->HANDLER->FAILURE, ...'
-
-        Note that this describes the last *completed* connection attempt. If
-        a connection attempt is currently in progress, this method will
-        describe the results of the previous attempt.
-        """)
-
     last_received_time = Attribute(
         """
         A timestamp (seconds-since-epoch) describing the last time we heard
@@ -2887,3 +2872,14 @@ class IConnectionStatus(Interface):
         the other side.
         """)
 
+    non_connected_statuses = Attribute(
+        """
+        A dictionary, describing all connections that are not (yet)
+        successful. When connected is True, this will only be the losing
+        attempts. When connected is False, this will include all attempts.
+
+        This maps a connection description string (for foolscap this is a
+        connection hint and the handler it is using) to the status string
+        (pending, connected, refused, or other errors).
+        """)
+

src/allmydata/test/test_connections.py

@@ -345,11 +345,13 @@ class Privacy(unittest.TestCase):
         self.assertEqual(str(e), "tub.location includes tcp: hint")
 
 class Status(unittest.TestCase):
-    def test_describe(self):
+    def test_hint_statuses(self):
-        t = connection_status._describe_statuses(["h2","h1"],
+        ncs = connection_status._hint_statuses(["h2","h1"],
-                                                 {"h1": "hand1"},
+                                               {"h1": "hand1", "h4": "hand4"},
-                                                 {"h1": "st1", "h2": "st2"})
+                                               {"h1": "st1", "h2": "st2",
-        self.assertEqual(t, " h1 via hand1: st1\n h2: st2\n")
+                                                "h3": "st3"})
+        self.assertEqual(ncs, {"h1 via hand1": "st1",
+                               "h2": "st2"})
 
     def test_reconnector_connected(self):
         ci = mock.Mock()
@@ -364,10 +366,8 @@ class Status(unittest.TestCase):
         rc.getReconnectionInfo = mock.Mock(return_value=ri)
         cs = connection_status.from_foolscap_reconnector(rc, 123)
         self.assertEqual(cs.connected, True)
-        self.assertEqual(cs.last_connection_summary,
+        self.assertEqual(cs.summary, "Connected to h1 via hand1")
-                         "Connected to h1 via hand1")
+        self.assertEqual(cs.non_connected_statuses, {})
-        self.assertEqual(cs.last_connection_description,
-                         "Connection successful to h1 via hand1")
         self.assertEqual(cs.last_connection_time, 120)
         self.assertEqual(cs.last_received_time, 123)
 
@@ -384,12 +384,8 @@ class Status(unittest.TestCase):
         rc.getReconnectionInfo = mock.Mock(return_value=ri)
         cs = connection_status.from_foolscap_reconnector(rc, 123)
         self.assertEqual(cs.connected, True)
-        self.assertEqual(cs.last_connection_summary,
+        self.assertEqual(cs.summary, "Connected to h1 via hand1")
-                         "Connected to h1 via hand1")
+        self.assertEqual(cs.non_connected_statuses, {"h2": "st2"})
-        self.assertEqual(cs.last_connection_description,
-                         "Connection successful to h1 via hand1\n"
-                         "other hints:\n"
-                         " h2: st2\n")
         self.assertEqual(cs.last_connection_time, 120)
         self.assertEqual(cs.last_received_time, 123)
 
@@ -407,13 +403,9 @@ class Status(unittest.TestCase):
         rc.getReconnectionInfo = mock.Mock(return_value=ri)
         cs = connection_status.from_foolscap_reconnector(rc, 123)
         self.assertEqual(cs.connected, True)
-        self.assertEqual(cs.last_connection_summary,
+        self.assertEqual(cs.summary, "Connected via listener (listener1)")
-                         "Connected via listener (listener1)")
+        self.assertEqual(cs.non_connected_statuses,
-        self.assertEqual(cs.last_connection_description,
+                         {"h1 via hand1": "st1", "h2": "st2"})
-                         "Connection successful via listener (listener1)\n"
-                         "other hints:\n"
-                         " h1 via hand1: st1\n"
-                         " h2: st2\n")
         self.assertEqual(cs.last_connection_time, 120)
         self.assertEqual(cs.last_received_time, 123)
 
@@ -428,12 +420,9 @@ class Status(unittest.TestCase):
         rc.getReconnectionInfo = mock.Mock(return_value=ri)
         cs = connection_status.from_foolscap_reconnector(rc, 123)
         self.assertEqual(cs.connected, False)
-        self.assertEqual(cs.last_connection_summary,
+        self.assertEqual(cs.summary, "Trying to connect")
-                         "Trying to connect")
+        self.assertEqual(cs.non_connected_statuses,
-        self.assertEqual(cs.last_connection_description,
+                         {"h1 via hand1": "st1", "h2": "st2"})
-                         "Trying to connect:\n"
-                         " h1 via hand1: st1\n"
-                         " h2: st2\n")
         self.assertEqual(cs.last_connection_time, None)
         self.assertEqual(cs.last_received_time, 123)
 
@@ -451,13 +440,10 @@ class Status(unittest.TestCase):
         with mock.patch("time.time", return_value=12):
             cs = connection_status.from_foolscap_reconnector(rc, 5)
         self.assertEqual(cs.connected, False)
-        self.assertEqual(cs.last_connection_summary,
+        self.assertEqual(cs.summary,
-                         "Reconnecting in 8 seconds")
+                         "Reconnecting in 8 seconds (last attempt 2s ago)")
-        self.assertEqual(cs.last_connection_description,
+        self.assertEqual(cs.non_connected_statuses,
-                         "Reconnecting in 8 seconds\n"
+                         {"h1 via hand1": "st1", "h2": "st2"})
-                         "Last attempt 2s ago:\n"
-                         " h1 via hand1: st1\n"
-                         " h2: st2\n")
         self.assertEqual(cs.last_connection_time, None)
         self.assertEqual(cs.last_received_time, 5)
 

src/allmydata/test/web/test_root.py

@@ -27,7 +27,7 @@ class RenderServiceRow(unittest.TestCase):
                "permutation-seed-base32": "w2hqnbaa25yw4qgcvghl5psa3srpfgw3",
                }
         s = NativeStorageServer("server_id", ann, None, {})
-        cs = ConnectionStatus(False, "summary", "description", 0, 0)
+        cs = ConnectionStatus(False, "summary", {}, 0, 0)
         s.get_connection_status = lambda: cs
 
         r = FakeRoot()

src/allmydata/test/web/test_web.py

@@ -178,7 +178,7 @@ class FakeDisplayableServer(StubServer):
     def get_available_space(self):
         return 123456
     def get_connection_status(self):
-        return ConnectionStatus(self.connected, "summary", "description",
+        return ConnectionStatus(self.connected, "summary", {},
                                 self.last_connect_time, self.last_rx_time)
 
 class FakeBucketCounter(object):
@@ -667,8 +667,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
             def __init__(self, connected):
                 self.connected = connected
             def connection_status(self):
-                return ConnectionStatus(self.connected,
+                return ConnectionStatus(self.connected, "summary", {}, 0, 0)
-                                        "summary", "description", 0, 0)
 
         d = defer.succeed(None)
 
@@ -680,7 +679,6 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         d.addCallback(_set_introducer_not_connected_unguessable)
         def _check_introducer_not_connected_unguessable(res):
             html = res.replace('\n', ' ')
-            self.failUnlessIn('<div class="connection-status" title="description">summary</div>', html)
             self.failIfIn('pb://someIntroducer/secret', html)
             self.failUnless(re.search('<img (alt="Disconnected" |src="img/connected-no.png" ){2}/></div>[ ]*<div>No introducers connected</div>', html), res)
 
@@ -694,7 +692,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         d.addCallback(_set_introducer_connected_unguessable)
         def _check_introducer_connected_unguessable(res):
             html = res.replace('\n', ' ')
-            self.failUnlessIn('<div class="connection-status" title="description">summary</div>', html)
+            self.failUnlessIn('<div class="connection-status" title="(no other hints)">summary</div>', html)
             self.failIfIn('pb://someIntroducer/secret', html)
             self.failUnless(re.search('<img (src="img/connected-yes.png" |alt="Connected" ){2}/></div>[ ]*<div>1 introducer connected</div>', html), res)
         d.addCallback(_check_introducer_connected_unguessable)
@@ -707,7 +705,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         d.addCallback(_set_introducer_connected_guessable)
         def _check_introducer_connected_guessable(res):
             html = res.replace('\n', ' ')
-            self.failUnlessIn('<div class="connection-status" title="description">summary</div>', html)
+            self.failUnlessIn('<div class="connection-status" title="(no other hints)">summary</div>', html)
             self.failUnless(re.search('<img (src="img/connected-yes.png" |alt="Connected" ){2}/></div>[ ]*<div>1 introducer connected</div>', html), res)
         d.addCallback(_check_introducer_connected_guessable)
         return d

src/allmydata/util/connection_status.py

@@ -4,24 +4,22 @@ from ..interfaces import IConnectionStatus
 
 @implementer(IConnectionStatus)
 class ConnectionStatus:
-    def __init__(self, connected, summary,
+    def __init__(self, connected, summary, non_connected_statuses,
-                 last_connection_description, last_connection_time,
+                 last_connection_time, last_received_time):
-                 last_received_time, statuses):
         self.connected = connected
-        self.last_connection_summary = summary
+        self.summary = summary
-        self.last_connection_description = last_connection_description
+        self.non_connected_statuses = non_connected_statuses
         self.last_connection_time = last_connection_time
         self.last_received_time = last_received_time
-        self.statuses = statuses
 
-def _describe_statuses(hints, handlers, statuses):
+def _hint_statuses(which, handlers, statuses):
-    descriptions = []
+    non_connected_statuses = {}
-    for hint in sorted(hints):
+    for hint in which:
         handler = handlers.get(hint)
         handler_dsc = " via %s" % handler if handler else ""
-        status = statuses[hint]
+        dsc = statuses[hint]
-        descriptions.append(" %s%s: %s\n" % (hint, handler_dsc, status))
+        non_connected_statuses["%s%s" % (hint, handler_dsc)] = dsc
-    return "".join(descriptions)
+    return non_connected_statuses
 
 def from_foolscap_reconnector(rc, last_received):
     ri = rc.getReconnectionInfo()
@@ -30,53 +28,33 @@ def from_foolscap_reconnector(rc, last_received):
     # should never see "unstarted"
     assert state in ("connected", "connecting", "waiting"), state
     ci = ri.connectionInfo
+    connected = False
+    last_connected = None
+    others = set(ci.connectorStatuses.keys())
 
     if state == "connected":
         connected = True
-        # build a description that shows the winning hint, and the outcomes
+        if ci.winningHint:
-        # of the losing ones
+            others.remove(ci.winningHint)
-        statuses = ci.connectorStatuses
+            summary = "Connected to %s via %s" % (
-        handlers = ci.connectionHandlers
+                ci.winningHint, ci.connectionHandlers[ci.winningHint])
-        others = set(statuses.keys())
-
-        winner = ci.winningHint
-        if winner:
-            others.remove(winner)
-            winning_handler = ci.connectionHandlers[winner]
-            winning_dsc = "to %s via %s" % (winner, winning_handler)
         else:
-            winning_dsc = "via listener (%s)" % ci.listenerStatus[0]
+            summary = "Connected via listener (%s)" % ci.listenerStatus[0]
-        if others:
-            other_dsc = "\nother hints:\n%s" % \
-                        _describe_statuses(others, handlers, statuses)
-        else:
-            other_dsc = ""
-        details = "Connection successful " + winning_dsc + other_dsc
-        summary = "Connected %s" % winning_dsc
         last_connected = ci.establishedAt
     elif state == "connecting":
-        connected = False
         # ci describes the current in-progress attempt
-        statuses = ci.connectorStatuses
-        current = _describe_statuses(sorted(statuses.keys()),
-                                     ci.connectionHandlers, statuses)
-        details = "Trying to connect:\n%s" % current
         summary = "Trying to connect"
-        last_connected = None
     elif state == "waiting":
-        connected = False
         now = time.time()
         elapsed = now - ri.lastAttempt
         delay = ri.nextAttempt - now
+        summary = "Reconnecting in %d seconds (last attempt %ds ago)" % \
+                  (delay, elapsed)
         # ci describes the previous (failed) attempt
-        statuses = ci.connectorStatuses
-        last = _describe_statuses(sorted(statuses.keys()),
-                                  ci.connectionHandlers, statuses)
-        details = "Reconnecting in %d seconds\nLast attempt %ds ago:\n%s" \
-                  % (delay, elapsed, last)
-        summary = "Reconnecting in %d seconds" % delay
-        last_connected = None
 
-    cs = ConnectionStatus(connected, summary, details,
+    non_connected_statuses = _hint_statuses(others,
-                          last_connected, last_received, statuses)
+                                            ci.connectionHandlers,
+                                            ci.connectorStatuses)
+    cs = ConnectionStatus(connected, summary, non_connected_statuses,
+                          last_connected, last_received)
     return cs

src/allmydata/web/root.py

@@ -1,14 +1,11 @@
 import time, os
-from datetime import datetime
 
 from twisted.web import http
 from nevow import rend, url, tags as T
-from nevow.inevow import IRequest, IContainer
+from nevow.inevow import IRequest
 from nevow.static import File as nevow_File # TODO: merge with static.File?
 from nevow.util import resource_filename
 
-from zope.interface import implementer
-
 import allmydata # to display import path
 from allmydata import get_package_versions_string
 from allmydata.util import log
@@ -17,7 +14,6 @@ from allmydata.web import filenode, directory, unlinked, status, operations
 from allmydata.web import storage, magic_folder
 from allmydata.web.common import abbreviate_size, getxmlfile, WebError, \
      get_arg, RenderMixin, get_format, get_mutable_type, render_time_delta, render_time, render_time_attr
-from allmydata.util.abbreviate import abbreviate_time
 
 
 class URIHandler(RenderMixin, rend.Page):
@@ -261,7 +257,7 @@ class Root(rend.Page):
     def data_introducers(self, ctx, data):
         return self.client.introducer_connection_statuses()
 
-    def render_introducers_row(self, ctx, cs):
+    def _render_connection_status(self, ctx, cs):
         connected = "yes" if cs.connected else "no"
         ctx.fillSlots("service_connection_status", connected)
         ctx.fillSlots("service_connection_status_alt",
@@ -286,8 +282,25 @@ class Root(rend.Page):
                       render_time_delta(last_received_data_time, self.now_fn())
                       if last_received_data_time is not None
                       else "N/A")
-        ctx.fillSlots("summary", "%s" % cs.last_connection_summary)
+
-        ctx.fillSlots("details", "%s" % cs.last_connection_description)
+        others = cs.non_connected_statuses
+        if cs.connected:
+            ctx.fillSlots("summary", cs.summary)
+            if others:
+                details = "\n".join(["* %s: %s\n" % (which, others[which])
+                                     for which in sorted(others)])
+                ctx.fillSlots("details", "Other hints:\n" + details)
+            else:
+                ctx.fillSlots("details", "(no other hints)")
+        else:
+            details = T.ul()
+            for which in sorted(others):
+                details[T.li["%s: %s" % (which, others[which])]]
+            ctx.fillSlots("summary", [cs.summary, details])
+            ctx.fillSlots("details", "")
+
+    def render_introducers_row(self, ctx, cs):
+        self._render_connection_status(ctx, cs)
         return ctx.tag
 
     def data_helper_furl_prefix(self, ctx, data):
@@ -333,75 +346,15 @@ class Root(rend.Page):
 
     def data_services(self, ctx, data):
         sb = self.client.get_storage_broker()
-
+        return sorted(sb.get_known_servers(), key=lambda s: s.get_serverid())
-        @implementer(IContainer)
-        class Wrapper(object):
-            """
-            This provides IContainer to Nevow so we can provide a 'child' data
-            at 'connections' and pass-through all the
-            connection-status attributes to the rest of the renderers
-            """
-            def __init__(self, server):
-                self._server = server
-
-            def child(self, context, name):
-                if name == 'connections':
-                    st = self._server.get_connection_status()
-                    if st.connected:
-                        # we don't want the list of all possible hints
-                        # when we're already connected; then we just
-                        # show the one connection
-                        return []
-                    return st.statuses.items()
-                return None  # or are we supposed to raise something?
-
-            def __getattr__(self, x):
-                return getattr(self._server, x)
-
-        return [
-            Wrapper(x)
-            for x in sorted(sb.get_known_servers(), key=lambda s: s.get_serverid())
-        ]
-
-    def render_connection_item(self, ctx, server):
-        ctx.tag.fillSlots('details', server[1])
-        ctx.tag.fillSlots('summary', '{}: {}'.format(server[0], server[1]))
-        return ctx.tag
 
     def render_service_row(self, ctx, server):
         cs = server.get_connection_status()
+        self._render_connection_status(ctx, cs)
 
         ctx.fillSlots("peerid", server.get_longname())
         ctx.fillSlots("nickname", server.get_nickname())
 
-        connected = "yes" if cs.connected else "no"
-        ctx.fillSlots("service_connection_status", connected)
-        ctx.fillSlots("service_connection_status_alt",
-                      self._connectedalts[connected])
-
-        since = cs.last_connection_time
-        ctx.fillSlots("service_connection_status_rel_time",
-                      render_time_delta(since, self.now_fn())
-                      if since is not None
-                      else "N/A")
-        ctx.fillSlots("service_connection_status_abs_time",
-                      render_time_attr(since)
-                      if since is not None
-                      else "N/A")
-
-        last_received_data_time = cs.last_received_time
-        ctx.fillSlots("last_received_data_abs_time",
-                      render_time_attr(last_received_data_time)
-                      if last_received_data_time is not None
-                      else "N/A")
-        ctx.fillSlots("last_received_data_rel_time",
-                      render_time_delta(last_received_data_time, self.now_fn())
-                      if last_received_data_time is not None
-                      else "N/A")
-
-        ctx.fillSlots("summary", "%s" % cs.last_connection_summary)
-        ctx.fillSlots("details", "%s" % cs.last_connection_description)
-
         announcement = server.get_announcement()
         version = announcement.get("my-version", "")
         available_space = server.get_available_space()

src/allmydata/web/welcome.xhtml

@@ -188,20 +188,16 @@
                 <div class="nickname"><n:slot name="nickname"/></div>
                 <div class="nodeid"><n:slot name="peerid"/></div>
               </td>
-              <td class="connection-status">
 
+              <td class="connection-status">
                 <n:attr name="title"><n:slot name="details"/></n:attr>
                 <n:slot name="summary"/>
                 <a class="timestamp">
                   <n:attr name="title"><n:slot name="service_connection_status_abs_time"/></n:attr>
                   <n:slot name="service_connection_status_rel_time"/>
                 </a>
-
-                <ul n:render="sequence" n:data="connections" class="details">
-                  <li n:pattern="item" n:render="connection_item"><n:attr name="title"><n:slot name="details"/></n:attr><n:slot name="summary"/></li>
-                </ul>
-
               </td>
+
               <td class="service-last-received-data"><a class="timestamp"><n:attr name="title"><n:slot name="last_received_data_abs_time"/></n:attr><n:slot name="last_received_data_rel_time"/></a></td>
               <td class="service-version"><n:slot name="version"/></td>
               <td class="service-available-space"><n:slot name="available_space"/></td>