CheckResults: pass IServer to sharemap=, but get_sharemap() returns serverids

This changes all code which feeds CheckResults(sharemap=) to provide
IServer instances, but CheckResults converts these to old-style
serverids during output, so downstream code doesn't have to change yet.

It adds a temporary get_new_sharemap(), which *does* return IServer
instances, so the immutable repairer can build new CheckResults from an
old one. This will go away when get_sharemap() is updated to return
IServer (and downstream code is updated too).

src/allmydata/check_results.py

@@ -1,8 +1,8 @@
 
 from zope.interface import implements
 from allmydata.interfaces import ICheckResults, ICheckAndRepairResults, \
-     IDeepCheckResults, IDeepCheckAndRepairResults, IURI
+     IDeepCheckResults, IDeepCheckAndRepairResults, IURI, IDisplayableServer
-from allmydata.util import base32
+from allmydata.util import base32, dictutil
 
 class CheckResults:
     implements(ICheckResults)
@@ -42,9 +42,9 @@ class CheckResults:
         for s in servers_responding:
             assert isinstance(s, str), s
         self._servers_responding = servers_responding
-        for shnum, serverids in sharemap.items():
+        for shnum, servers in sharemap.items():
-            for serverid in serverids:
+            for server in servers:
-                assert isinstance(serverid, str), serverid
+                assert IDisplayableServer.providedBy(server), server
         self._sharemap = sharemap
         self._count_wrong_shares = count_wrong_shares
         for (serverid, SI, shnum) in list_corrupt_shares:
@@ -109,9 +109,19 @@ 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):
+        sharemap = {}
+        for shnum, servers in self._sharemap.items():
+            sharemap[shnum] = sorted([s.get_serverid() for s in servers])
         d = {"count-shares-needed": self._count_shares_needed,
              "count-shares-expected": self._count_shares_expected,
              "count-shares-good": self._count_shares_good,
@@ -119,7 +129,7 @@ class CheckResults:
              "count-recoverable-versions": self._count_recoverable_versions,
              "count-unrecoverable-versions": self._count_unrecoverable_versions,
              "servers-responding": self._servers_responding,
-             "sharemap": self._sharemap,
+             "sharemap": sharemap,
              "count-wrong-shares": self._count_wrong_shares,
              "list-corrupt-shares": self._list_corrupt_shares,
              "count-corrupt-shares": self._count_corrupt_shares,

src/allmydata/immutable/checker.py

@@ -739,7 +739,7 @@ class Checker(log.PrefixingLogMixin):
     def _format_results(self, results):
         SI = self._verifycap.get_storage_index()
 
-        verifiedshares = dictutil.DictOfSets() # {sharenum: set(serverid)}
+        verifiedshares = dictutil.DictOfSets() # {sharenum: set(server)}
         servers = {} # {serverid: set(sharenums)}
         corruptshare_locators = [] # (serverid, storageindex, sharenum)
         incompatibleshare_locators = [] # (serverid, storageindex, sharenum)
@@ -749,7 +749,7 @@ class Checker(log.PrefixingLogMixin):
             server_id = server.get_serverid()
             servers.setdefault(server_id, set()).update(verified)
             for sharenum in verified:
-                verifiedshares.setdefault(sharenum, set()).add(server_id)
+                verifiedshares.setdefault(sharenum, set()).add(server)
             for sharenum in corrupt:
                 corruptshare_locators.append((server_id, SI, sharenum))
             for sharenum in incompatible:

src/allmydata/immutable/filenode.py

@@ -129,13 +129,13 @@ class CiphertextFileNode:
         servers_responding = set(cr.get_servers_responding())
         sm = DictOfSets()
         assert isinstance(cr.get_sharemap(), DictOfSets)
-        for shnum, serverids in cr.get_sharemap().items():
+        for shnum, servers in cr.get_new_sharemap().items():
-            for serverid in serverids:
+            for server in servers:
-                sm.add(shnum, serverid)
+                sm.add(shnum, server)
         for shnum, servers in ur.get_sharemap().items():
-            for s in servers:
+            for server in servers:
-                sm.add(shnum, s.get_serverid())
+                sm.add(shnum, server)
-                servers_responding.add(s.get_serverid())
+                servers_responding.add(server.get_serverid())
         servers_responding = sorted(servers_responding)
 
         good_hosts = len(reduce(set.union, sm.values(), set()))

src/allmydata/mutable/checker.py

@@ -1,6 +1,6 @@
 
 from allmydata.uri import from_string
-from allmydata.util import base32, log
+from allmydata.util import base32, log, dictutil
 from allmydata.check_results import CheckAndRepairResults, CheckResults
 
 from allmydata.mutable.common import MODE_CHECK, MODE_WRITE, CorruptShareError
@@ -213,13 +213,11 @@ class MutableChecker:
                     where=ft,
                     level=log.WEIRD, umid="EkK8QA")
 
-        sharemap = {}
+        sharemap = dictutil.DictOfSets()
         for verinfo in vmap:
             for (shnum, server, timestamp) in vmap[verinfo]:
                 shareid = "%s-sh%d" % (smap.summarize_version(verinfo), shnum)
-                if shareid not in sharemap:
+                sharemap.add(shareid, server)
-                    sharemap[shareid] = []
-                sharemap[shareid].append(server.get_serverid())
         servers_responding = [s.get_serverid() for s in
                               list(smap.get_reachable_servers())]
         if healthy:

src/allmydata/test/common.py

@@ -14,6 +14,7 @@ from allmydata.interfaces import IMutableFileNode, IImmutableFileNode,\
                                  MDMF_VERSION
 from allmydata.check_results import CheckResults, CheckAndRepairResults, \
      DeepCheckResults, DeepCheckAndRepairResults
+from allmydata.storage_client import StubServer
 from allmydata.mutable.layout import unpack_header
 from allmydata.mutable.publish import MutableData
 from allmydata.storage.mutable import MutableShareFile
@@ -67,6 +68,7 @@ class FakeCHKFileNode:
 
     def check(self, monitor, verify=False, add_lease=False):
         nodeid = "\x00"*20
+        s = StubServer(nodeid)
         r = CheckResults(self.my_uri, self.storage_index,
                          healthy=True, recoverable=True,
                          needs_rebalancing=False,
@@ -77,7 +79,7 @@ class FakeCHKFileNode:
                          count_recoverable_versions=1,
                          count_unrecoverable_versions=0,
                          servers_responding=[nodeid],
-                         sharemap={1: [nodeid]},
+                         sharemap={1: [s]},
                          count_wrong_shares=0,
                          list_corrupt_shares=[],
                          count_corrupt_shares=0,
@@ -279,6 +281,7 @@ class FakeMutableFileNode:
 
     def check(self, monitor, verify=False, add_lease=False):
         nodeid = "\x00"*20
+        s = StubServer(nodeid)
         r = CheckResults(self.my_uri, self.storage_index,
                          healthy=True, recoverable=True,
                          needs_rebalancing=False,
@@ -289,7 +292,7 @@ class FakeMutableFileNode:
                          count_recoverable_versions=1,
                          count_unrecoverable_versions=0,
                          servers_responding=[nodeid],
-                         sharemap={"seq1-abcd-sh0": [nodeid]},
+                         sharemap={"seq1-abcd-sh0": [s]},
                          count_wrong_shares=0,
                          list_corrupt_shares=[],
                          count_corrupt_shares=0,

src/allmydata/test/test_checker.py

@@ -78,6 +78,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
 
     def test_check(self):
         c = self.create_fake_client()
+        sb = c.storage_broker
         serverid_1 = "\x00"*20
         serverid_f = "\xff"*20
         u = uri.CHKFileURI("\x00"*16, "\x00"*32, 3, 10, 1234)
@@ -88,7 +89,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
                  "count_recoverable_versions": 1,
                  "count_unrecoverable_versions": 0,
                  "servers_responding": [],
-                 "sharemap": {"shareid1": [serverid_1, serverid_f]},
+                 "sharemap": {"shareid1": [sb.get_stub_server(serverid_1),
+                                           sb.get_stub_server(serverid_f)]},
                  "count_wrong_shares": 0,
                  "list_corrupt_shares": [],
                  "count_corrupt_shares": 0,
@@ -159,8 +161,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
                         'count-unrecoverable-versions': 0,
                         'count-shares-needed': 3,
                         'sharemap': {"shareid1":
-                                     ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+                                     ["77777777777777777777777777777777",
-                                      "77777777777777777777777777777777"]},
+                                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]},
                         'count-recoverable-versions': 1,
                         'list-corrupt-shares':
                         [["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
@@ -185,6 +187,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
 
     def test_check_and_repair(self):
         c = self.create_fake_client()
+        sb = c.storage_broker
         serverid_1 = "\x00"*20
         serverid_f = "\xff"*20
         u = uri.CHKFileURI("\x00"*16, "\x00"*32, 3, 10, 1234)
@@ -196,7 +199,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
                  "count_recoverable_versions": 1,
                  "count_unrecoverable_versions": 0,
                  "servers_responding": [],
-                 "sharemap": {"shareid1": [serverid_1, serverid_f]},
+                 "sharemap": {"shareid1": [sb.get_stub_server(serverid_1),
+                                           sb.get_stub_server(serverid_f)]},
                  "count_wrong_shares": 0,
                  "list_corrupt_shares": [],
                  "count_corrupt_shares": 0,
@@ -217,7 +221,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
                  "count_recoverable_versions": 1,
                  "count_unrecoverable_versions": 0,
                  "servers_responding": [],
-                 "sharemap": {"shareid1": [serverid_1, serverid_f]},
+                 "sharemap": {"shareid1": [sb.get_stub_server(serverid_1),
+                                           sb.get_stub_server(serverid_f)]},
                  "count_wrong_shares": 0,
                  "count_corrupt_shares": 0,
                  "list_corrupt_shares": [],