From: Brian Warner <warner@lothar.com>
Date: Fri, 25 May 2012 19:58:02 +0000 (-0700)
Subject: CheckResults.get_sharemap() now returns IServers
X-Git-Url: https://git.rkrishnan.org/pf/content/en/seg/bcase?a=commitdiff_plain;h=957a5315aa8f1a67efb2d72e57934518bfb25184;p=tahoe-lafs%2Ftahoe-lafs.git

CheckResults.get_sharemap() now returns IServers

Remove temporary get_new_sharemap().
---

diff --git a/src/allmydata/check_results.py b/src/allmydata/check_results.py
index d0c2cda3..e9de13ca 100644
--- a/src/allmydata/check_results.py
+++ b/src/allmydata/check_results.py
@@ -2,7 +2,7 @@
 from zope.interface import implements
 from allmydata.interfaces import ICheckResults, ICheckAndRepairResults, \
      IDeepCheckResults, IDeepCheckAndRepairResults, IURI, IDisplayableServer
-from allmydata.util import base32, dictutil
+from allmydata.util import base32
 
 class CheckResults:
     implements(ICheckResults)
@@ -117,13 +117,6 @@ class CheckResults:
         return self._count_unrecoverable_versions
 
     def get_sharemap(self):
-        sharemap = dictutil.DictOfSets()
-        for shnum, servers in self._sharemap.items():
-            for s in servers:
-                sharemap.add(shnum, s.get_serverid())
-        return sharemap
-    def get_new_sharemap(self):
-        # this one returns IServers, even when get_sharemap returns serverids
         return self._sharemap
 
     def as_dict(self):
diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py
index 578445fb..e248dc9b 100644
--- a/src/allmydata/immutable/filenode.py
+++ b/src/allmydata/immutable/filenode.py
@@ -129,7 +129,7 @@ class CiphertextFileNode:
         servers_responding = set(cr.get_new_servers_responding())
         sm = DictOfSets()
         assert isinstance(cr.get_sharemap(), DictOfSets)
-        for shnum, servers in cr.get_new_sharemap().items():
+        for shnum, servers in cr.get_sharemap().items():
             for server in servers:
                 sm.add(shnum, server)
         for shnum, servers in ur.get_sharemap().items():
diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 4439d84e..f4fa2a9d 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -2191,12 +2191,11 @@ class ICheckResults(Interface):
          healthy file, this will be 0."""
 
     def get_sharemap():
-        """Return a dict mapping share identifier to list of serverids
-        (binary strings). This indicates which servers are holding which
-        shares. For immutable files, the shareid is an integer (the share
-        number, from 0 to N-1). For mutable files, it is a string of the form
-        'seq%d-%s-sh%d', containing the sequence number, the roothash, and
-        the share number."""
+        """Return a dict mapping share identifier to list of IServer objects.
+        This indicates which servers are holding which shares. For immutable
+        files, the shareid is an integer (the share number, from 0 to N-1).
+        For mutable files, it is a string of the form 'seq%d-%s-sh%d',
+        containing the sequence number, the roothash, and the share number."""
 
     def get_summary():
         """Return a string with a brief (one-line) summary of the results."""
diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py
index e1a57544..d83025ad 100644
--- a/src/allmydata/test/test_checker.py
+++ b/src/allmydata/test/test_checker.py
@@ -115,6 +115,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
         self.failUnlessIn("Wrong Shares: 0", s)
         self.failUnlessIn("Recoverable Versions: 1", s)
         self.failUnlessIn("Unrecoverable Versions: 0", s)
+        self.failUnlessIn("Good Shares (sorted in share order): Share ID Nickname Node ID shareid1 peer-0 00000000 peer-f ffffffff", s)
 
         cr = check_results.CheckResults(u, u.get_storage_index(),
                                         healthy=False, recoverable=True,
@@ -162,8 +163,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
                         'count-unrecoverable-versions': 0,
                         'count-shares-needed': 3,
                         'sharemap': {"shareid1":
-                                     ["77777777777777777777777777777777",
-                                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]},
+                                     ["v0-00000000-long", "v0-ffffffff-long"]},
                         'count-recoverable-versions': 1,
                         'list-corrupt-shares':
                         [["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
diff --git a/src/allmydata/test/test_deepcheck.py b/src/allmydata/test/test_deepcheck.py
index 86415b29..2ce8382e 100644
--- a/src/allmydata/test/test_deepcheck.py
+++ b/src/allmydata/test/test_deepcheck.py
@@ -293,8 +293,8 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
                                  sorted(self.g.get_all_serverids()),
                                  where)
             all_serverids = set()
-            for (shareid, serverids) in cr.get_sharemap().items():
-                all_serverids.update(serverids)
+            for (shareid, servers) in cr.get_sharemap().items():
+                all_serverids.update([s.get_serverid() for s in servers])
             self.failUnlessEqual(sorted(all_serverids),
                                  sorted(self.g.get_all_serverids()),
                                  where)
diff --git a/src/allmydata/web/check_results.py b/src/allmydata/web/check_results.py
index a0ebf3d3..f1f3b3c8 100644
--- a/src/allmydata/web/check_results.py
+++ b/src/allmydata/web/check_results.py
@@ -6,7 +6,7 @@ from twisted.web import http, html
 from allmydata.web.common import getxmlfile, get_arg, get_root, WebError
 from allmydata.web.operations import ReloadMixin
 from allmydata.interfaces import ICheckAndRepairResults, ICheckResults
-from allmydata.util import base32, idlib
+from allmydata.util import base32, idlib, dictutil
 
 def json_check_counts(r):
     d = {"count-shares-good": r.get_share_counter_good(),
@@ -20,9 +20,9 @@ def json_check_counts(r):
                                   in r.get_corrupt_shares() ],
          "servers-responding": [idlib.nodeid_b2a(serverid)
                                 for serverid in r.get_servers_responding()],
-         "sharemap": dict([(shareid, [idlib.nodeid_b2a(serverid)
-                                      for serverid in serverids])
-                           for (shareid, serverids)
+         "sharemap": dict([(shareid,
+                            sorted([s.get_longname() for s in servers]))
+                           for (shareid, servers)
                            in r.get_sharemap().items()]),
          "count-wrong-shares": r.get_share_counter_wrong(),
          "count-recoverable-versions": r.get_version_counter_recoverable(),
@@ -106,50 +106,49 @@ class ResultsBase:
 
         add("Wrong Shares", cr.get_share_counter_wrong())
 
-        sharemap = []
-        servers = {}
+        sharemap_data = []
+        shares_on_server = dictutil.DictOfSets()
 
         # FIXME: The two tables below contain nickname-and-nodeid table column markup which is duplicated with each other, introducer.xhtml, and deep-check-results.xhtml. All of these (and any other presentations of nickname-and-nodeid) should be combined.
 
         for shareid in sorted(cr.get_sharemap().keys()):
-            serverids = cr.get_sharemap()[shareid]
-            for i,serverid in enumerate(serverids):
-                if serverid not in servers:
-                    servers[serverid] = []
-                servers[serverid].append(shareid)
+            servers = sorted(cr.get_sharemap()[shareid],
+                             key=lambda s: s.get_longname())
+            for i,s in enumerate(servers):
+                shares_on_server.add(s, shareid)
                 shareid_s = ""
                 if i == 0:
                     shareid_s = shareid
-                nickname = sb.get_nickname_for_serverid(serverid)
-                sharemap.append(T.tr[T.td[shareid_s],
-                                     T.td[T.div(class_="nickname")[nickname],
-                                          T.div(class_="nodeid")[T.tt[base32.b2a(serverid)]]]
-                                     ])
+                d = T.tr[T.td[shareid_s],
+                         T.td[T.div(class_="nickname")[s.get_nickname()],
+                              T.div(class_="nodeid")[T.tt[s.get_name()]]]
+                         ]
+                sharemap_data.append(d)
         add("Good Shares (sorted in share order)",
             T.table()[T.tr[T.th["Share ID"], T.th(class_="nickname-and-peerid")[T.div["Nickname"], T.div(class_="nodeid")["Node ID"]]],
-                      sharemap])
+                      sharemap_data])
 
 
         add("Recoverable Versions", cr.get_version_counter_recoverable())
         add("Unrecoverable Versions", cr.get_version_counter_unrecoverable())
 
         # this table is sorted by permuted order
-        sb = c.get_storage_broker()
         permuted_servers = [s
                             for s
                             in sb.get_servers_for_psi(cr.get_storage_index())]
 
-        num_shares_left = sum([len(shares) for shares in servers.values()])
+        num_shares_left = sum([len(shareids)
+                               for shareids in shares_on_server.values()])
         servermap = []
         for s in permuted_servers:
-            nickname = s.get_nickname()
-            shareids = servers.get(s.get_serverid(), [])
+            shareids = list(shares_on_server.get(s, []))
             shareids.reverse()
             shareids_s = [ T.tt[shareid, " "] for shareid in sorted(shareids) ]
-            servermap.append(T.tr[T.td[T.div(class_="nickname")[nickname],
-                                       T.div(class_="nodeid")[T.tt[s.get_name()]]],
-                                  T.td[shareids_s],
-                                  ])
+            d = T.tr[T.td[T.div(class_="nickname")[s.get_nickname()],
+                          T.div(class_="nodeid")[T.tt[s.get_name()]]],
+                     T.td[shareids_s],
+                     ]
+            servermap.append(d)
             num_shares_left -= len(shareids)
             if not num_shares_left:
                 break