From 4867dca3f04a4c7590b07b8c9f51fee422076b1c Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 25 May 2012 00:13:48 -0700
Subject: [PATCH] use the new CheckResult getters almost everywhere

The remaining get_data() calls are either in
web.check_results.json_check_results(), or functioning as repr()s in
various unit test failure cases.
---
 src/allmydata/check_results.py       |  6 +--
 src/allmydata/immutable/filenode.py  | 21 +++++----
 src/allmydata/test/test_checker.py   |  6 +--
 src/allmydata/test/test_deepcheck.py | 51 +++++++++-----------
 src/allmydata/test/test_repairer.py  | 70 ++++++++++++----------------
 src/allmydata/web/check_results.py   | 23 +++++----
 6 files changed, 81 insertions(+), 96 deletions(-)

diff --git a/src/allmydata/check_results.py b/src/allmydata/check_results.py
index 1517f96a..922b6c15 100644
--- a/src/allmydata/check_results.py
+++ b/src/allmydata/check_results.py
@@ -196,7 +196,7 @@ class DeepCheckResults(DeepResultsBase):
             self.objects_unrecoverable += 1
         self.all_results[tuple(path)] = r
         self.all_results_by_storage_index[r.get_storage_index()] = r
-        self.corrupt_shares.extend(r.get_data()["list-corrupt-shares"])
+        self.corrupt_shares.extend(r.get_corrupt_shares())
 
     def get_counters(self):
         return {"count-objects-checked": self.objects_checked,
@@ -234,7 +234,7 @@ class DeepCheckAndRepairResults(DeepResultsBase):
             self.objects_unhealthy += 1
         if not pre_repair.is_recoverable():
             self.objects_unrecoverable += 1
-        self.corrupt_shares.extend(pre_repair.get_data()["list-corrupt-shares"])
+        self.corrupt_shares.extend(pre_repair.get_corrupt_shares())
         if r.get_repair_attempted():
             self.repairs_attempted += 1
             if r.get_repair_successful():
@@ -249,7 +249,7 @@ class DeepCheckAndRepairResults(DeepResultsBase):
             self.objects_unrecoverable_post_repair += 1
         self.all_results[tuple(path)] = r
         self.all_results_by_storage_index[r.get_storage_index()] = r
-        self.corrupt_shares_post_repair.extend(post_repair.get_data()["list-corrupt-shares"])
+        self.corrupt_shares_post_repair.extend(post_repair.get_corrupt_shares())
 
     def get_counters(self):
         return {"count-objects-checked": self.objects_checked,
diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py
index d8735189..0c9b719a 100644
--- a/src/allmydata/immutable/filenode.py
+++ b/src/allmydata/immutable/filenode.py
@@ -1,6 +1,5 @@
 
 import binascii
-import copy
 import time
 now = time.time
 from zope.interface import implements
@@ -126,19 +125,21 @@ class CiphertextFileNode:
         # clone the cr (check results) to form the basis of the
         # prr (post-repair results)
         prr = CheckResults(cr.uri, cr.storage_index)
-        prr_data = copy.deepcopy(cr.get_data())
 
         verifycap = self._verifycap
-        servers_responding = set(prr_data['servers-responding'])
-        sm = prr_data['sharemap']
-        assert isinstance(sm, DictOfSets), sm
+        servers_responding = set(cr.get_servers_responding())
+        sm = DictOfSets()
+        assert isinstance(cr.get_sharemap(), DictOfSets)
+        for shnum, serverids in cr.get_sharemap().items():
+            for serverid in serverids:
+                sm.add(shnum, serverid)
         for shnum, servers in ur.get_sharemap().items():
             for s in servers:
                 sm.add(shnum, s.get_serverid())
                 servers_responding.add(s.get_serverid())
         servers_responding = sorted(servers_responding)
 
-        good_hosts = len(reduce(set.union, sm.itervalues(), set()))
+        good_hosts = len(reduce(set.union, sm.values(), set()))
         is_healthy = bool(len(sm) >= verifycap.total_shares)
         is_recoverable = bool(len(sm) >= verifycap.needed_shares)
         prr.set_data(
@@ -151,10 +152,10 @@ class CiphertextFileNode:
             servers_responding=list(servers_responding),
             sharemap=sm,
             count_wrong_shares=0, # no such thing as wrong, for immutable
-            list_corrupt_shares=prr_data["list-corrupt-shares"],
-            count_corrupt_shares=prr_data["count-corrupt-shares"],
-            list_incompatible_shares=prr_data["list-incompatible-shares"],
-            count_incompatible_shares=prr_data["count-incompatible-shares"],
+            list_corrupt_shares=cr.get_corrupt_shares(),
+            count_corrupt_shares=len(cr.get_corrupt_shares()),
+            list_incompatible_shares=cr.get_incompatible_shares(),
+            count_incompatible_shares=len(cr.get_incompatible_shares()),
             )
         prr.set_healthy(is_healthy)
         prr.set_recoverable(is_recoverable)
diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py
index 585a3e43..d378860e 100644
--- a/src/allmydata/test/test_checker.py
+++ b/src/allmydata/test/test_checker.py
@@ -376,10 +376,10 @@ class BalancingAct(GridTestMixin, unittest.TestCase):
         def _check_and_repair(_):
             return self.imm.check_and_repair(Monitor())
         def _check_counts(crr, shares_good, good_share_hosts):
-            p_crr = crr.get_post_repair_results().get_data()
+            prr = crr.get_post_repair_results()
             #print self._pretty_shares_chart(self.uri)
-            self.failUnlessEqual(p_crr['count-shares-good'], shares_good)
-            self.failUnlessEqual(p_crr['count-good-share-hosts'],
+            self.failUnlessEqual(prr.get_share_counter_good(), shares_good)
+            self.failUnlessEqual(prr.get_host_counter_good_shares(),
                                  good_share_hosts)
 
         """
diff --git a/src/allmydata/test/test_deepcheck.py b/src/allmydata/test/test_deepcheck.py
index 669a6d5a..ac50fc73 100644
--- a/src/allmydata/test/test_deepcheck.py
+++ b/src/allmydata/test/test_deepcheck.py
@@ -281,30 +281,27 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase):
         if not incomplete:
             self.failUnlessEqual(cr.needs_rebalancing(), needs_rebalancing,
                                  str((where, cr, cr.get_data())))
-        d = cr.get_data()
-        self.failUnlessEqual(d["count-shares-good"], 10, where)
-        self.failUnlessEqual(d["count-shares-needed"], 3, where)
-        self.failUnlessEqual(d["count-shares-expected"], 10, where)
+        self.failUnlessEqual(cr.get_share_counter_good(), 10, where)
+        self.failUnlessEqual(cr.get_encoding_needed(), 3, where)
+        self.failUnlessEqual(cr.get_encoding_expected(), 10, where)
         if not incomplete:
-            self.failUnlessEqual(d["count-good-share-hosts"], num_servers,
-                                 where)
-        self.failUnlessEqual(d["count-corrupt-shares"], 0, where)
-        self.failUnlessEqual(d["list-corrupt-shares"], [], where)
+            self.failUnlessEqual(cr.get_host_counter_good_shares(),
+                                 num_servers, where)
+        self.failUnlessEqual(cr.get_corrupt_shares(), [], where)
         if not incomplete:
-            self.failUnlessEqual(sorted(d["servers-responding"]),
+            self.failUnlessEqual(sorted(cr.get_servers_responding()),
                                  sorted(self.g.get_all_serverids()),
                                  where)
-            self.failUnless("sharemap" in d, str((where, d)))
             all_serverids = set()
-            for (shareid, serverids) in d["sharemap"].items():
+            for (shareid, serverids) in cr.get_sharemap().items():
                 all_serverids.update(serverids)
             self.failUnlessEqual(sorted(all_serverids),
                                  sorted(self.g.get_all_serverids()),
                                  where)
 
-        self.failUnlessEqual(d["count-wrong-shares"], 0, where)
-        self.failUnlessEqual(d["count-recoverable-versions"], 1, where)
-        self.failUnlessEqual(d["count-unrecoverable-versions"], 0, where)
+        self.failUnlessEqual(cr.get_share_counter_wrong(), 0, where)
+        self.failUnlessEqual(cr.get_version_counter_recoverable(), 1, where)
+        self.failUnlessEqual(cr.get_version_counter_unrecoverable(), 0, where)
 
 
     def check_and_repair_is_healthy(self, cr, n, where, incomplete=False):
@@ -1010,9 +1007,8 @@ class DeepCheckWebBad(DeepCheckBase, unittest.TestCase):
             self.failUnless(ICheckResults.providedBy(cr), (cr, type(cr), where))
             self.failUnless(cr.is_healthy(), (cr.get_report(), cr.is_healthy(), cr.get_summary(), where))
             self.failUnless(cr.is_recoverable(), where)
-            d = cr.get_data()
-            self.failUnlessEqual(d["count-recoverable-versions"], 1, where)
-            self.failUnlessEqual(d["count-unrecoverable-versions"], 0, where)
+            self.failUnlessEqual(cr.get_version_counter_recoverable(), 1, where)
+            self.failUnlessEqual(cr.get_version_counter_unrecoverable(), 0, where)
             return cr
         except Exception, le:
             le.args = tuple(le.args + (where,))
@@ -1022,31 +1018,28 @@ class DeepCheckWebBad(DeepCheckBase, unittest.TestCase):
         self.failUnless(ICheckResults.providedBy(cr), where)
         self.failIf(cr.is_healthy(), where)
         self.failUnless(cr.is_recoverable(), where)
-        d = cr.get_data()
-        self.failUnlessEqual(d["count-recoverable-versions"], 1, where)
-        self.failUnlessEqual(d["count-unrecoverable-versions"], 0, where)
+        self.failUnlessEqual(cr.get_version_counter_recoverable(), 1, where)
+        self.failUnlessEqual(cr.get_version_counter_unrecoverable(), 0, where)
         return cr
 
     def check_has_corrupt_shares(self, cr, where):
         # by "corrupt-shares" we mean the file is still recoverable
         self.failUnless(ICheckResults.providedBy(cr), where)
-        d = cr.get_data()
         self.failIf(cr.is_healthy(), (where, cr))
         self.failUnless(cr.is_recoverable(), where)
-        d = cr.get_data()
-        self.failUnless(d["count-shares-good"] < 10, where)
-        self.failUnless(d["count-corrupt-shares"], where)
-        self.failUnless(d["list-corrupt-shares"], where)
+        self.failUnless(cr.get_share_counter_good() < 10, where)
+        self.failUnless(cr.get_corrupt_shares(), where)
         return cr
 
     def check_is_unrecoverable(self, cr, where):
         self.failUnless(ICheckResults.providedBy(cr), where)
-        d = cr.get_data()
         self.failIf(cr.is_healthy(), where)
         self.failIf(cr.is_recoverable(), where)
-        self.failUnless(d["count-shares-good"] < d["count-shares-needed"], (d["count-shares-good"], d["count-shares-needed"], where))
-        self.failUnlessEqual(d["count-recoverable-versions"], 0, where)
-        self.failUnlessEqual(d["count-unrecoverable-versions"], 1, where)
+        self.failUnless(cr.get_share_counter_good() < cr.get_encoding_needed(),
+                        (cr.get_share_counter_good(), cr.get_encoding_needed(),
+                         where))
+        self.failUnlessEqual(cr.get_version_counter_recoverable(), 0, where)
+        self.failUnlessEqual(cr.get_version_counter_unrecoverable(), 1, where)
         return cr
 
     def do_check(self, ignored):
diff --git a/src/allmydata/test/test_repairer.py b/src/allmydata/test/test_repairer.py
index 531db0d1..60b28e24 100644
--- a/src/allmydata/test/test_repairer.py
+++ b/src/allmydata/test/test_repairer.py
@@ -128,14 +128,13 @@ class Verifier(GridTestMixin, unittest.TestCase, RepairTestMixin):
         touched in a way that matters. It doesn't use more than seven times
         as many reads as it needs."""
         self.failUnless(vr.is_healthy(), (vr, vr.is_healthy(), vr.get_data()))
-        data = vr.get_data()
-        self.failUnless(data['count-shares-good'] == 10, data)
-        self.failUnless(len(data['sharemap']) == 10, data)
-        self.failUnless(data['count-shares-needed'] == 3, data)
-        self.failUnless(data['count-shares-expected'] == 10, data)
-        self.failUnless(data['count-good-share-hosts'] == 10, data)
-        self.failUnless(len(data['servers-responding']) == 10, data)
-        self.failUnless(len(data['list-corrupt-shares']) == 0, data)
+        self.failUnlessEqual(vr.get_share_counter_good(), 10)
+        self.failUnlessEqual(len(vr.get_sharemap()), 10)
+        self.failUnlessEqual(vr.get_encoding_needed(), 3)
+        self.failUnlessEqual(vr.get_encoding_expected(), 10)
+        self.failUnlessEqual(vr.get_host_counter_good_shares(), 10)
+        self.failUnlessEqual(len(vr.get_servers_responding()), 10)
+        self.failUnlessEqual(len(vr.get_corrupt_shares()), 0)
 
     def test_ok_no_corruption(self):
         self.basedir = "repairer/Verifier/ok_no_corruption"
@@ -164,14 +163,13 @@ class Verifier(GridTestMixin, unittest.TestCase, RepairTestMixin):
         correctly. It doesn't use more than seven times as many reads as it
         needs."""
         self.failIf(vr.is_healthy(), (vr, vr.is_healthy(), vr.get_data()))
-        data = vr.get_data()
-        self.failUnless(data['count-shares-good'] == 9, data)
-        self.failUnless(len(data['sharemap']) == 9, data)
-        self.failUnless(data['count-shares-needed'] == 3, data)
-        self.failUnless(data['count-shares-expected'] == 10, data)
-        self.failUnless(data['count-good-share-hosts'] == 9, data)
-        self.failUnless(len(data['servers-responding']) == 9, data)
-        self.failUnless(len(data['list-corrupt-shares']) == 0, data)
+        self.failUnlessEqual(vr.get_share_counter_good(), 9)
+        self.failUnlessEqual(len(vr.get_sharemap()), 9)
+        self.failUnlessEqual(vr.get_encoding_needed(), 3)
+        self.failUnlessEqual(vr.get_encoding_expected(), 10)
+        self.failUnlessEqual(vr.get_host_counter_good_shares(), 9)
+        self.failUnlessEqual(len(vr.get_servers_responding()), 9)
+        self.failUnlessEqual(len(vr.get_corrupt_shares()), 0)
 
     def test_corrupt_file_verno(self):
         self.basedir = "repairer/Verifier/corrupt_file_verno"
@@ -185,17 +183,14 @@ class Verifier(GridTestMixin, unittest.TestCase, RepairTestMixin):
         # ShareVersionIncompatible exception, which should be counted in
         # list-incompatible-shares, rather than list-corrupt-shares.
         self.failIf(vr.is_healthy(), (vr, vr.is_healthy(), vr.get_data()))
-        data = vr.get_data()
-        self.failUnlessEqual(data['count-shares-good'], 9)
-        self.failUnlessEqual(len(data['sharemap']), 9)
-        self.failUnlessEqual(data['count-shares-needed'], 3)
-        self.failUnlessEqual(data['count-shares-expected'], 10)
-        self.failUnlessEqual(data['count-good-share-hosts'], 9)
-        self.failUnlessEqual(len(data['servers-responding']), 10)
-        self.failUnlessEqual(len(data['list-corrupt-shares']), 0)
-        self.failUnlessEqual(data['count-corrupt-shares'], 0)
-        self.failUnlessEqual(len(data['list-incompatible-shares']), 1)
-        self.failUnlessEqual(data['count-incompatible-shares'], 1)
+        self.failUnlessEqual(vr.get_share_counter_good(), 9)
+        self.failUnlessEqual(len(vr.get_sharemap()), 9)
+        self.failUnlessEqual(vr.get_encoding_needed(), 3)
+        self.failUnlessEqual(vr.get_encoding_expected(), 10)
+        self.failUnlessEqual(vr.get_host_counter_good_shares(), 9)
+        self.failUnlessEqual(len(vr.get_servers_responding()), 10)
+        self.failUnlessEqual(len(vr.get_corrupt_shares()), 0)
+        self.failUnlessEqual(len(vr.get_incompatible_shares()), 1)
 
     def test_corrupt_share_verno(self):
         self.basedir = "repairer/Verifier/corrupt_share_verno"
@@ -207,17 +202,14 @@ class Verifier(GridTestMixin, unittest.TestCase, RepairTestMixin):
         # of them), which will be detected by the client as it downloads
         # those shares.
         self.failIf(vr.is_healthy(), (vr, vr.is_healthy(), vr.get_data()))
-        data = vr.get_data()
-        self.failUnlessEqual(data['count-shares-good'], 9)
-        self.failUnlessEqual(data['count-shares-needed'], 3)
-        self.failUnlessEqual(data['count-shares-expected'], 10)
-        self.failUnlessEqual(data['count-good-share-hosts'], 9)
-        self.failUnlessEqual(data['count-corrupt-shares'], 1)
-        self.failUnlessEqual(len(data['list-corrupt-shares']), 1)
-        self.failUnlessEqual(data['count-incompatible-shares'], 0)
-        self.failUnlessEqual(len(data['list-incompatible-shares']), 0)
-        self.failUnlessEqual(len(data['servers-responding']), 10)
-        self.failUnlessEqual(len(data['sharemap']), 9)
+        self.failUnlessEqual(vr.get_share_counter_good(), 9)
+        self.failUnlessEqual(vr.get_encoding_needed(), 3)
+        self.failUnlessEqual(vr.get_encoding_expected(), 10)
+        self.failUnlessEqual(vr.get_host_counter_good_shares(), 9)
+        self.failUnlessEqual(len(vr.get_corrupt_shares()), 1)
+        self.failUnlessEqual(len(vr.get_incompatible_shares()), 0)
+        self.failUnlessEqual(len(vr.get_servers_responding()), 10)
+        self.failUnlessEqual(len(vr.get_sharemap()), 9)
 
     def test_corrupt_sharedata_offset(self):
         self.basedir = "repairer/Verifier/corrupt_sharedata_offset"
@@ -724,7 +716,7 @@ class Repairer(GridTestMixin, unittest.TestCase, RepairTestMixin,
             # not respond to the pre-repair filecheck
             prr = rr.get_post_repair_results()
             expected = set(self.g.get_all_serverids())
-            self.failUnlessEqual(expected, set(prr.get_data()["servers-responding"]))
+            self.failUnlessEqual(expected, set(prr.get_servers_responding()))
         d.addCallback(_check)
         return d
 
diff --git a/src/allmydata/web/check_results.py b/src/allmydata/web/check_results.py
index 39524a96..3d381544 100644
--- a/src/allmydata/web/check_results.py
+++ b/src/allmydata/web/check_results.py
@@ -81,21 +81,20 @@ class ResultsBase:
         assert ICheckResults(cr)
         c = self.client
         sb = c.get_storage_broker()
-        data = cr.get_data()
         r = []
         def add(name, value):
             r.append(T.li[name + ": ", value])
 
         add("Report", T.pre["\n".join(self._html(cr.get_report()))])
         add("Share Counts",
-            "need %d-of-%d, have %d" % (data["count-shares-needed"],
-                                        data["count-shares-expected"],
-                                        data["count-shares-good"]))
-        add("Hosts with good shares", data["count-good-share-hosts"])
+            "need %d-of-%d, have %d" % (cr.get_encoding_needed(),
+                                        cr.get_encoding_expected(),
+                                        cr.get_share_counter_good()))
+        add("Hosts with good shares", cr.get_host_counter_good_shares())
 
-        if data["list-corrupt-shares"]:
+        if cr.get_corrupt_shares():
             badsharemap = []
-            for (serverid, si, shnum) in data["list-corrupt-shares"]:
+            for (serverid, si, shnum) in cr.get_corrupt_shares():
                 nickname = sb.get_nickname_for_serverid(serverid)
                 badsharemap.append(T.tr[T.td["sh#%d" % shnum],
                                         T.td[T.div(class_="nickname")[nickname],
@@ -108,15 +107,15 @@ class ResultsBase:
         else:
             add("Corrupt shares", "none")
 
-        add("Wrong Shares", data["count-wrong-shares"])
+        add("Wrong Shares", cr.get_share_counter_wrong())
 
         sharemap = []
         servers = {}
 
         # 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(data["sharemap"].keys()):
-            serverids = data["sharemap"][shareid]
+        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] = []
@@ -134,8 +133,8 @@ class ResultsBase:
                       sharemap])
 
 
-        add("Recoverable Versions", data["count-recoverable-versions"])
-        add("Unrecoverable Versions", data["count-unrecoverable-versions"])
+        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()
-- 
2.45.2