From 76fca000dfc6e9f1bba54969bb554214d2533f80 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 25 May 2012 12:57:53 -0700
Subject: [PATCH] CheckResults: pass IServer to corrupt/incompatible share
 locators

Getters still return serverid. Adds temporary get_new_corrupt_shares()
and get_new_incompatible_shares().
---
 src/allmydata/check_results.py      | 26 ++++++++++++++++++--------
 src/allmydata/immutable/checker.py  | 11 +++++------
 src/allmydata/immutable/filenode.py |  4 ++--
 src/allmydata/mutable/checker.py    |  2 +-
 src/allmydata/test/test_checker.py  |  7 ++++---
 5 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/allmydata/check_results.py b/src/allmydata/check_results.py
index bfda6bcd..d0c2cda3 100644
--- a/src/allmydata/check_results.py
+++ b/src/allmydata/check_results.py
@@ -47,12 +47,12 @@ class CheckResults:
                 assert IDisplayableServer.providedBy(server), server
         self._sharemap = sharemap
         self._count_wrong_shares = count_wrong_shares
-        for (serverid, SI, shnum) in list_corrupt_shares:
-            assert isinstance(serverid, str), serverid
+        for (server, SI, shnum) in list_corrupt_shares:
+            assert IDisplayableServer.providedBy(server), server
         self._list_corrupt_shares = list_corrupt_shares
         self._count_corrupt_shares = count_corrupt_shares
-        for (serverid, SI, shnum) in list_incompatible_shares:
-            assert isinstance(serverid, str), serverid
+        for (server, SI, shnum) in list_incompatible_shares:
+            assert IDisplayableServer.providedBy(server), server
         self._list_incompatible_shares = list_incompatible_shares
         self._count_incompatible_shares = count_incompatible_shares
 
@@ -91,11 +91,17 @@ class CheckResults:
     def get_share_counter_wrong(self):
         return self._count_wrong_shares
 
-    def get_corrupt_shares(self):
+    def get_new_corrupt_shares(self):
         return self._list_corrupt_shares
+    def get_corrupt_shares(self):
+        return [(s.get_serverid(), SI, shnum)
+                for (s, SI, shnum) in self._list_corrupt_shares]
 
-    def get_incompatible_shares(self):
+    def get_new_incompatible_shares(self):
         return self._list_incompatible_shares
+    def get_incompatible_shares(self):
+        return [(s.get_serverid(), SI, shnum)
+                for (s, SI, shnum) in self._list_incompatible_shares]
 
     def get_new_servers_responding(self):
         return self._servers_responding
@@ -125,6 +131,10 @@ class CheckResults:
         for shnum, servers in self._sharemap.items():
             sharemap[shnum] = sorted([s.get_serverid() for s in servers])
         responding = [s.get_serverid() for s in self._servers_responding]
+        corrupt = [(s.get_serverid(), SI, shnum)
+                   for (s, SI, shnum) in self._list_corrupt_shares]
+        incompatible = [(s.get_serverid(), SI, shnum)
+                        for (s, SI, shnum) in self._list_incompatible_shares]
         d = {"count-shares-needed": self._count_shares_needed,
              "count-shares-expected": self._count_shares_expected,
              "count-shares-good": self._count_shares_good,
@@ -134,9 +144,9 @@ class CheckResults:
              "servers-responding": responding,
              "sharemap": sharemap,
              "count-wrong-shares": self._count_wrong_shares,
-             "list-corrupt-shares": self._list_corrupt_shares,
+             "list-corrupt-shares": corrupt,
              "count-corrupt-shares": self._count_corrupt_shares,
-             "list-incompatible-shares": self._list_incompatible_shares,
+             "list-incompatible-shares": incompatible,
              "count-incompatible-shares": self._count_incompatible_shares,
              }
         return d
diff --git a/src/allmydata/immutable/checker.py b/src/allmydata/immutable/checker.py
index 6e882b57..37d6a5ea 100644
--- a/src/allmydata/immutable/checker.py
+++ b/src/allmydata/immutable/checker.py
@@ -741,19 +741,18 @@ class Checker(log.PrefixingLogMixin):
 
         verifiedshares = dictutil.DictOfSets() # {sharenum: set(server)}
         servers = {} # {server: set(sharenums)}
-        corruptshare_locators = [] # (serverid, storageindex, sharenum)
-        incompatibleshare_locators = [] # (serverid, storageindex, sharenum)
-        servers_responding = set() # serverid
+        corruptshare_locators = [] # (server, storageindex, sharenum)
+        incompatibleshare_locators = [] # (server, storageindex, sharenum)
+        servers_responding = set() # server
 
         for verified, server, corrupt, incompatible, responded in results:
-            server_id = server.get_serverid()
             servers.setdefault(server, set()).update(verified)
             for sharenum in verified:
                 verifiedshares.setdefault(sharenum, set()).add(server)
             for sharenum in corrupt:
-                corruptshare_locators.append((server_id, SI, sharenum))
+                corruptshare_locators.append((server, SI, sharenum))
             for sharenum in incompatible:
-                incompatibleshare_locators.append((server_id, SI, sharenum))
+                incompatibleshare_locators.append((server, SI, sharenum))
             if responded:
                 servers_responding.add(server)
 
diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py
index b54d3ded..578445fb 100644
--- a/src/allmydata/immutable/filenode.py
+++ b/src/allmydata/immutable/filenode.py
@@ -154,9 +154,9 @@ class CiphertextFileNode:
                            servers_responding=list(servers_responding),
                            sharemap=sm,
                            count_wrong_shares=0, # no such thing as wrong, for immutable
-                           list_corrupt_shares=cr.get_corrupt_shares(),
+                           list_corrupt_shares=cr.get_new_corrupt_shares(),
                            count_corrupt_shares=len(cr.get_corrupt_shares()),
-                           list_incompatible_shares=cr.get_incompatible_shares(),
+                           list_incompatible_shares=cr.get_new_incompatible_shares(),
                            count_incompatible_shares=len(cr.get_incompatible_shares()),
                            summary="",
                            report=[],
diff --git a/src/allmydata/mutable/checker.py b/src/allmydata/mutable/checker.py
index 978b6ac3..08580f18 100644
--- a/src/allmydata/mutable/checker.py
+++ b/src/allmydata/mutable/checker.py
@@ -193,7 +193,7 @@ class MutableChecker:
             summary.append("Corrupt Shares:")
         for (server, shnum, f) in sorted(self.bad_shares):
             serverid = server.get_serverid()
-            locator = (serverid, self._storage_index, shnum)
+            locator = (server, self._storage_index, shnum)
             corrupt_share_locators.append(locator)
             s = "%s-sh%d" % (server.get_name(), shnum)
             if f.check(CorruptShareError):
diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py
index 1946c6c5..e1a57544 100644
--- a/src/allmydata/test/test_checker.py
+++ b/src/allmydata/test/test_checker.py
@@ -81,6 +81,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
         sb = c.storage_broker
         serverid_1 = "\x00"*20
         serverid_f = "\xff"*20
+        server_1 = sb.get_stub_server(serverid_1)
+        server_f = sb.get_stub_server(serverid_f)
         u = uri.CHKFileURI("\x00"*16, "\x00"*32, 3, 10, 1234)
         data = { "count_shares_needed": 3,
                  "count_shares_expected": 9,
@@ -89,8 +91,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
                  "count_recoverable_versions": 1,
                  "count_unrecoverable_versions": 0,
                  "servers_responding": [],
-                 "sharemap": {"shareid1": [sb.get_stub_server(serverid_1),
-                                           sb.get_stub_server(serverid_f)]},
+                 "sharemap": {"shareid1": [server_1, server_f]},
                  "count_wrong_shares": 0,
                  "list_corrupt_shares": [],
                  "count_corrupt_shares": 0,
@@ -127,7 +128,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
         self.failUnlessIn("Not Healthy! : ungroovy", s)
 
         data["count_corrupt_shares"] = 1
-        data["list_corrupt_shares"] = [(serverid_1, u.get_storage_index(), 2)]
+        data["list_corrupt_shares"] = [(server_1, u.get_storage_index(), 2)]
         cr = check_results.CheckResults(u, u.get_storage_index(),
                                         healthy=False, recoverable=False,
                                         needs_rebalancing=False,
-- 
2.45.2