From f570ad7ba5da6a9f4062c1b86e0bdb48df164225 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@allmydata.com>
Date: Wed, 10 Sep 2008 13:44:58 -0700
Subject: [PATCH] disallow deep-check on non-directories, simplifies the code a
 bit

---
 docs/webapi.txt                     | 10 +++++---
 src/allmydata/dirnode.py            |  4 +--
 src/allmydata/immutable/filenode.py | 28 ---------------------
 src/allmydata/interfaces.py         |  9 +++----
 src/allmydata/mutable/node.py       | 21 ----------------
 src/allmydata/test/test_filenode.py | 39 -----------------------------
 src/allmydata/test/test_system.py   | 36 ++------------------------
 7 files changed, 15 insertions(+), 132 deletions(-)

diff --git a/docs/webapi.txt b/docs/webapi.txt
index cfc3611d..b558fda7 100644
--- a/docs/webapi.txt
+++ b/docs/webapi.txt
@@ -729,9 +729,9 @@ POST $URL?t=deep-check
   page will contain a summary of the results, including details on any
   file/directory that was not fully healthy.
 
-  t=deep-check is most useful to invoke on a directory. If invoked on a file,
-  it will just check that single object. The recursive walker will deal with
-  loops safely.
+  t=deep-check can only be invoked on a directory. An error (400 BAD_REQUEST)
+  will be signalled if it is invoked on a file. The recursive walker will
+  deal with loops safely.
 
   This accepts the same verify=, when_done=, and return_to= arguments as
   t=check.
@@ -801,6 +801,10 @@ POST $URL?t=deep-check&repair=true
   This triggers a recursive walk of all files and directories, performing a
   t=check&repair=true on each one.
 
+  Like t=deep-check without the repair= argument, this can only be invoked on
+  a directory. An error (400 BAD_REQUEST) will be signalled if it is invoked
+  on a file. The recursive walker will deal with loops safely.
+
   This accepts the same when_done=URL, return_to=URL, and verify=true
   arguments as t=deep-check. When an output=JSON argument is provided, the
   response will contain the following keys:
diff --git a/src/allmydata/dirnode.py b/src/allmydata/dirnode.py
index 3b3fbaee..6945bdec 100644
--- a/src/allmydata/dirnode.py
+++ b/src/allmydata/dirnode.py
@@ -8,7 +8,7 @@ from allmydata.mutable.common import NotMutableError
 from allmydata.mutable.node import MutableFileNode
 from allmydata.interfaces import IMutableFileNode, IDirectoryNode,\
      IURI, IFileNode, IMutableFileURI, IFilesystemNode, \
-     ExistingChildError, ICheckable
+     ExistingChildError, ICheckable, IDeepCheckable
 from allmydata.checker_results import DeepCheckResults, \
      DeepCheckAndRepairResults
 from allmydata.util import hashutil, mathutil, base32, log
@@ -114,7 +114,7 @@ class Adder:
         return new_contents
 
 class NewDirectoryNode:
-    implements(IDirectoryNode, ICheckable)
+    implements(IDirectoryNode, ICheckable, IDeepCheckable)
     filenode_class = MutableFileNode
 
     def __init__(self, client):
diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py
index 4f55819a..810e5e3f 100644
--- a/src/allmydata/immutable/filenode.py
+++ b/src/allmydata/immutable/filenode.py
@@ -5,8 +5,6 @@ from allmydata.interfaces import IFileNode, IFileURI, IURI, ICheckable
 from allmydata import uri
 from allmydata.immutable.checker import SimpleCHKFileChecker, \
      SimpleCHKFileVerifier
-from allmydata.checker_results import DeepCheckResults, \
-     DeepCheckAndRepairResults
 
 class FileNode:
     implements(IFileNode, ICheckable)
@@ -75,24 +73,6 @@ class FileNode:
         d.addCallback(_done)
         return d
 
-    def deep_check(self, verify=False):
-        d = self.check(verify)
-        def _done(r):
-            dr = DeepCheckResults(self.get_verifier().storage_index)
-            dr.add_check(r, [])
-            return dr
-        d.addCallback(_done)
-        return d
-
-    def deep_check_and_repair(self, verify=False):
-        d = self.check_and_repair(verify)
-        def _done(r):
-            dr = DeepCheckAndRepairResults(self.get_verifier().storage_index)
-            dr.add_check_and_repair(r, [])
-            return dr
-        d.addCallback(_done)
-        return d
-
     def download(self, target):
         downloader = self._client.getServiceNamed("downloader")
         return downloader.download(self.uri, target)
@@ -150,14 +130,6 @@ class LiteralFileNode:
     def check_and_repair(self, verify=False):
         return defer.succeed(None)
 
-    def deep_check(self, verify=False):
-        dr = DeepCheckResults(None)
-        return defer.succeed(dr)
-
-    def deep_check_and_repair(self, verify=False):
-        dr = DeepCheckAndRepairResults(None)
-        return defer.succeed(dr)
-
     def download(self, target):
         # note that this does not update the stats_provider
         data = IURI(self.uri).data
diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 44065ff0..c68ae11a 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -1485,11 +1485,11 @@ class ICheckable(Interface):
         attempted, 'post' will be another ICheckerResults instance with the
         state of the object after repair."""
 
+class IDeepCheckable(Interface):
     def deep_check(verify=False):
         """Check upon the health of me and everything I can reach.
 
-        This is a recursive form of check(), useable on dirnodes. (it can be
-        called safely on filenodes too, but only checks the one object).
+        This is a recursive form of check(), useable only on dirnodes.
 
         I return a Deferred that fires with an IDeepCheckResults object.
         """
@@ -1498,9 +1498,8 @@ class ICheckable(Interface):
         """Check upon the health of me and everything I can reach. Repair
         anything that isn't healthy.
 
-        This is a recursive form of check_and_repair(), useable on dirnodes.
-        (it can be called safely on filenodes too, but only checks/repairs
-        the one object).
+        This is a recursive form of check_and_repair(), useable only on
+        dirnodes.
 
         I return a Deferred that fires with an IDeepCheckAndRepairResults
         object.
diff --git a/src/allmydata/mutable/node.py b/src/allmydata/mutable/node.py
index 3ad4fd56..b70e0c15 100644
--- a/src/allmydata/mutable/node.py
+++ b/src/allmydata/mutable/node.py
@@ -12,8 +12,6 @@ from allmydata.util import hashutil
 from allmydata.util.assertutil import precondition
 from allmydata.uri import WriteableSSKFileURI
 from allmydata.immutable.encode import NotEnoughSharesError
-from allmydata.checker_results import DeepCheckResults, \
-     DeepCheckAndRepairResults
 from pycryptopp.publickey import rsa
 from pycryptopp.cipher.aes import AES
 
@@ -253,25 +251,6 @@ class MutableFileNode:
         checker = self.check_and_repairer_class(self)
         return checker.check(verify)
 
-    def deep_check(self, verify=False):
-        # deep-check on a filenode only gets one result
-        d = self.check(verify)
-        def _done(r):
-            dr = DeepCheckResults(self.get_storage_index())
-            dr.add_check(r, [])
-            return dr
-        d.addCallback(_done)
-        return d
-
-    def deep_check_and_repair(self, verify=False):
-        d = self.check_and_repair(verify)
-        def _done(r):
-            dr = DeepCheckAndRepairResults(self.get_storage_index())
-            dr.add_check_and_repair(r, [])
-            return dr
-        d.addCallback(_done)
-        return d
-
     #################################
     # IRepairable
 
diff --git a/src/allmydata/test/test_filenode.py b/src/allmydata/test/test_filenode.py
index 8ccdbd71..0f759194 100644
--- a/src/allmydata/test/test_filenode.py
+++ b/src/allmydata/test/test_filenode.py
@@ -134,19 +134,6 @@ class Checker(unittest.TestCase):
 
         # TODO: check-and-repair
 
-        d.addCallback(lambda res: fn1.deep_check())
-        def _check_deepcheck_results(dcr):
-            c = dcr.get_counters()
-            self.failUnlessEqual(c["count-objects-checked"], 1)
-            self.failUnlessEqual(c["count-objects-healthy"], 1)
-            self.failUnlessEqual(c["count-objects-unhealthy"], 0)
-            self.failUnlessEqual(c["count-corrupt-shares"], 0)
-            self.failIf(dcr.get_corrupt_shares())
-        d.addCallback(_check_deepcheck_results)
-
-        d.addCallback(lambda res: fn1.deep_check(verify=True))
-        d.addCallback(_check_deepcheck_results)
-
         return d
 
     def test_literal_filenode(self):
@@ -163,19 +150,6 @@ class Checker(unittest.TestCase):
         d.addCallback(lambda res: fn1.check(verify=True))
         d.addCallback(_check_checker_results)
 
-        d.addCallback(lambda res: fn1.deep_check())
-        def _check_deepcheck_results(dcr):
-            c = dcr.get_counters()
-            self.failUnlessEqual(c["count-objects-checked"], 0)
-            self.failUnlessEqual(c["count-objects-healthy"], 0)
-            self.failUnlessEqual(c["count-objects-unhealthy"], 0)
-            self.failUnlessEqual(c["count-corrupt-shares"], 0)
-            self.failIf(dcr.get_corrupt_shares())
-        d.addCallback(_check_deepcheck_results)
-
-        d.addCallback(lambda res: fn1.deep_check(verify=True))
-        d.addCallback(_check_deepcheck_results)
-
         return d
 
     def test_mutable_filenode(self):
@@ -199,19 +173,6 @@ class Checker(unittest.TestCase):
         d.addCallback(lambda res: n.check(verify=True))
         d.addCallback(_check_checker_results)
 
-        d.addCallback(lambda res: n.deep_check())
-        def _check_deepcheck_results(dcr):
-            c = dcr.get_counters()
-            self.failUnlessEqual(c["count-objects-checked"], 1)
-            self.failUnlessEqual(c["count-objects-healthy"], 1)
-            self.failUnlessEqual(c["count-objects-unhealthy"], 0)
-            self.failUnlessEqual(c["count-corrupt-shares"], 0)
-            self.failIf(dcr.get_corrupt_shares())
-        d.addCallback(_check_deepcheck_results)
-
-        d.addCallback(lambda res: n.deep_check(verify=True))
-        d.addCallback(_check_deepcheck_results)
-
         return d
 
 class FakeMutableChecker:
diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py
index 23618697..6f557a05 100644
--- a/src/allmydata/test/test_system.py
+++ b/src/allmydata/test/test_system.py
@@ -2162,46 +2162,15 @@ class DeepCheck(SystemTestMixin, unittest.TestCase):
         d.addCallback(self.failUnlessEqual, None, "small")
 
 
-        # now deep-check on everything. It should be safe to use deep-check
-        # on anything, even a regular file.
+        # now deep-check the root, with various verify= and repair= options
         d.addCallback(lambda ign: self.root.deep_check())
         d.addCallback(self.deep_check_is_healthy, 3, "root")
-        d.addCallback(lambda ign: self.mutable.deep_check())
-        d.addCallback(self.deep_check_is_healthy, 1, "mutable")
-        d.addCallback(lambda ign: self.large.deep_check())
-        d.addCallback(self.deep_check_is_healthy, 1, "large")
-        d.addCallback(lambda ign: self.small.deep_check())
-        d.addCallback(self.deep_check_is_healthy, 0, "small")
-
-        # deep-check verify=True
         d.addCallback(lambda ign: self.root.deep_check(verify=True))
         d.addCallback(self.deep_check_is_healthy, 3, "root")
-        d.addCallback(lambda ign: self.mutable.deep_check(verify=True))
-        d.addCallback(self.deep_check_is_healthy, 1, "mutable")
-        d.addCallback(lambda ign: self.large.deep_check(verify=True))
-        d.addCallback(self.deep_check_is_healthy, 1, "large")
-        d.addCallback(lambda ign: self.small.deep_check(verify=True))
-        d.addCallback(self.deep_check_is_healthy, 0, "small")
-
-        # deep-check-and-repair
         d.addCallback(lambda ign: self.root.deep_check_and_repair())
         d.addCallback(self.deep_check_and_repair_is_healthy, 3, "root")
-        d.addCallback(lambda ign: self.mutable.deep_check_and_repair())
-        d.addCallback(self.deep_check_and_repair_is_healthy, 1, "mutable")
-        d.addCallback(lambda ign: self.large.deep_check_and_repair())
-        d.addCallback(self.deep_check_and_repair_is_healthy, 1, "large")
-        d.addCallback(lambda ign: self.small.deep_check_and_repair())
-        d.addCallback(self.deep_check_and_repair_is_healthy, 0, "small")
-
-        # deep-check-and-repair, verify=True
         d.addCallback(lambda ign: self.root.deep_check_and_repair(verify=True))
         d.addCallback(self.deep_check_and_repair_is_healthy, 3, "root")
-        d.addCallback(lambda ign: self.mutable.deep_check_and_repair(verify=True))
-        d.addCallback(self.deep_check_and_repair_is_healthy, 1, "mutable")
-        d.addCallback(lambda ign: self.large.deep_check_and_repair(verify=True))
-        d.addCallback(self.deep_check_and_repair_is_healthy, 1, "large")
-        d.addCallback(lambda ign: self.small.deep_check_and_repair(verify=True))
-        d.addCallback(self.deep_check_and_repair_is_healthy, 0, "small")
 
         return d
 
@@ -2361,8 +2330,7 @@ class DeepCheck(SystemTestMixin, unittest.TestCase):
                       self.web_json(self.small, t="check", repair="true", verify="true"))
         d.addCallback(self.json_check_lit, self.small, "small")
 
-        # now run a deep-check. When done through the web, this can only be
-        # run on a directory.
+        # now run a deep-check, with various verify= and repair= flags
         d.addCallback(lambda ign:
                       self.web_json(self.root, t="deep-check"))
         d.addCallback(self.json_full_deepcheck_is_healthy, self.root, "root")
-- 
2.45.2