try to tidy up uri-as-string vs. uri-as-object
authorZooko O'Whielacronx <zooko@zooko.com>
Fri, 19 Dec 2008 15:39:24 +0000 (08:39 -0700)
committerZooko O'Whielacronx <zooko@zooko.com>
Fri, 19 Dec 2008 15:39:24 +0000 (08:39 -0700)
I get confused about whether a given argument or return value is a uri-as-string or uri-as-object.  This patch adds a lot of assertions that it is one or the other, and also changes CheckerResults to take objects not strings.
In the future, I hope that we generally use Python objects except when importing into or exporting from the Python interpreter e.g. over the wire, the UI, or a stored file.

src/allmydata/checker_results.py
src/allmydata/dirnode.py
src/allmydata/immutable/checker.py
src/allmydata/interfaces.py
src/allmydata/mutable/checker.py
src/allmydata/test/common.py
src/allmydata/test/test_dirnode.py
src/allmydata/test/test_web.py

index 7031ff3a4c8a292b923719dc2c9017f171cde93c..34c588e218621d540651779b43cac154ba0a68c6 100644 (file)
@@ -1,14 +1,14 @@
 
 from zope.interface import implements
 from allmydata.interfaces import ICheckerResults, ICheckAndRepairResults, \
-     IDeepCheckResults, IDeepCheckAndRepairResults
+     IDeepCheckResults, IDeepCheckAndRepairResults, IURI
 from allmydata.util import base32
 
 class CheckerResults:
     implements(ICheckerResults)
 
     def __init__(self, uri, storage_index):
-        assert isinstance(uri, str)
+        assert IURI.providedBy(uri), uri
         self.uri = uri
         self.storage_index = storage_index
         self.problems = []
index 38a311369812f96d514976b4f6ed94ab23295f06..1afa66558ef6defb0c560cc57acb687990b507a3 100644 (file)
@@ -13,6 +13,7 @@ from allmydata.checker_results import DeepCheckResults, \
      DeepCheckAndRepairResults
 from allmydata.monitor import Monitor
 from allmydata.util import hashutil, mathutil, base32, log
+from allmydata.util.assertutil import _assert, precondition
 from allmydata.util.hashutil import netstring
 from allmydata.util.limiter import ConcurrencyLimiter
 from allmydata.util.netstring import split_netstring
@@ -60,6 +61,8 @@ class Adder:
         self.overwrite = overwrite
 
     def set_node(self, name, node, metadata):
+        precondition(isinstance(name, unicode), name)
+        precondition(IFilesystemNode.providedBy(node), node)
         self.entries.append( [name, node, metadata] )
 
     def modify(self, old_contents, servermap, first_time):
@@ -72,6 +75,7 @@ class Adder:
             else:
                 assert len(e) == 3
                 name, child, new_metadata = e
+                assert _assert(IFilesystemNode.providedBy(child), child)
             assert isinstance(name, unicode)
             if name in children:
                 if not self.overwrite:
@@ -201,9 +205,9 @@ class NewDirectoryNode:
                     or IDirectoryNode.providedBy(child)), (name,child)
             assert isinstance(metadata, dict)
             rwcap = child.get_uri() # might be RO if the child is not writeable
+            assert isinstance(rwcap, str), rwcap
             rocap = child.get_readonly_uri()
             assert isinstance(rocap, str), rocap
-            assert isinstance(rwcap, str), rwcap
             entry = "".join([netstring(name.encode("utf-8")),
                              netstring(rocap),
                              netstring(self._encrypt_rwcap(rwcap)),
@@ -339,7 +343,8 @@ class NewDirectoryNode:
 
         If this directory node is read-only, the Deferred will errback with a
         NotMutableError."""
-        assert isinstance(name, unicode)
+        precondition(isinstance(name, unicode), name)
+        precondition(isinstance(child_uri, str), child_uri)
         child_node = self._create_node(child_uri)
         d = self.set_node(name, child_node, metadata, overwrite)
         d.addCallback(lambda res: child_node)
@@ -369,6 +374,8 @@ class NewDirectoryNode:
         If this directory node is read-only, the Deferred will errback with a
         NotMutableError."""
 
+        precondition(IFilesystemNode.providedBy(child), child)
+
         if self.is_readonly():
             return defer.fail(NotMutableError())
         assert isinstance(name, unicode)
index 615e0f0ae5d8355905f82d0b0d65acc7e2803533..8c63f9846874dadc9d3528f71cde030142f1bdb4 100644 (file)
@@ -70,7 +70,7 @@ class SimpleCHKFileChecker:
         pass
 
     def _done(self, res):
-        r = CheckerResults(self.uri.to_string(), self.storage_index)
+        r = CheckerResults(self.uri, self.storage_index)
         report = []
         healthy = bool(len(self.found_shares) >= self.total_shares)
         r.set_healthy(healthy)
@@ -179,7 +179,7 @@ class SimpleCHKFileVerifier(download.FileDownloader):
         self._si_s = storage.si_b2a(self._storage_index)
         self.init_logging()
 
-        self._check_results = r = CheckerResults(self._uri.to_string(), self._storage_index)
+        self._check_results = r = CheckerResults(self._uri, self._storage_index)
         r.set_data({"count-shares-needed": k,
                     "count-shares-expected": N,
                     })
index e87050ca5864427aa7f7a08344f0de8f728415ef..e348468846b84028ec7ba9185fad050a0d76d40a 100644 (file)
@@ -372,7 +372,7 @@ class IURI(Interface):
         """Return a string of printable ASCII characters, suitable for
         passing into init_from_string."""
 
-class IVerifierURI(Interface):
+class IVerifierURI(Interface, IURI):
     def init_from_string(uri):
         """Accept a string (as created by my to_string() method) and populate
         this instance with its data. I am not normally called directly,
index 1a64163b39a253155a522583c07ccdd190e8b243..31e8744c4a32fde48145c21d898303a9e61e6c7f 100644 (file)
@@ -2,6 +2,7 @@
 from twisted.internet import defer
 from twisted.python import failure
 from allmydata import hashtree
+from allmydata.uri import from_string
 from allmydata.util import hashutil, base32, idlib, log
 from allmydata.checker_results import CheckAndRepairResults, CheckerResults
 
@@ -16,7 +17,7 @@ class MutableChecker:
         self._monitor = monitor
         self.bad_shares = [] # list of (nodeid,shnum,failure)
         self._storage_index = self._node.get_storage_index()
-        self.results = CheckerResults(node.get_uri(), self._storage_index)
+        self.results = CheckerResults(from_string(node.get_uri()), self._storage_index)
         self.need_repair = False
         self.responded = set() # set of (binary) nodeids
 
@@ -297,7 +298,7 @@ class MutableCheckAndRepairer(MutableChecker):
         d = self._node.repair(self.results)
         def _repair_finished(repair_results):
             self.cr_results.repair_successful = True
-            r = CheckerResults(self._node.get_uri(), self._storage_index)
+            r = CheckerResults(from_string(self._node.get_uri()), self._storage_index)
             self.cr_results.post_repair_results = r
             self._fill_checker_results(repair_results.servermap, r)
             self.cr_results.repair_results = repair_results # TODO?
index 1d09f897e53fd84fbc74472bafa0e4382f95f27a..e21cccc80098c470b06f8e862082ca880545d73b 100644 (file)
@@ -16,6 +16,7 @@ from allmydata.checker_results import CheckerResults, CheckAndRepairResults, \
 from allmydata.mutable.common import CorruptShareError
 from allmydata.storage import storage_index_to_dir
 from allmydata.util import log, fileutil, pollmixin
+from allmydata.util.assertutil import precondition
 from allmydata.stats import StatsGathererService
 from allmydata.key_generator import KeyGeneratorService
 import common_util as testutil
@@ -36,16 +37,17 @@ class FakeCHKFileNode:
     bad_shares = {}
 
     def __init__(self, u, client):
+        precondition(IURI.providedBy(u), u)
         self.client = client
-        self.my_uri = u.to_string()
+        self.my_uri = u
         self.storage_index = u.storage_index
 
     def get_uri(self):
-        return self.my_uri
+        return self.my_uri.to_string()
     def get_readonly_uri(self):
-        return self.my_uri
+        return self.my_uri.to_string()
     def get_verify_cap(self):
-        return IURI(self.my_uri).get_verify_cap()
+        return self.my_uri.get_verify_cap()
     def get_storage_index(self):
         return self.storage_index
 
@@ -92,25 +94,25 @@ class FakeCHKFileNode:
         return True
 
     def download(self, target):
-        if self.my_uri not in self.all_contents:
+        if self.my_uri.to_string() not in self.all_contents:
             f = failure.Failure(NotEnoughSharesError())
             target.fail(f)
             return defer.fail(f)
-        data = self.all_contents[self.my_uri]
+        data = self.all_contents[self.my_uri.to_string()]
         target.open(len(data))
         target.write(data)
         target.close()
         return defer.maybeDeferred(target.finish)
     def download_to_data(self):
-        if self.my_uri not in self.all_contents:
+        if self.my_uri.to_string() not in self.all_contents:
             return defer.fail(NotEnoughSharesError())
-        data = self.all_contents[self.my_uri]
+        data = self.all_contents[self.my_uri.to_string()]
         return defer.succeed(data)
     def get_size(self):
         try:
-            data = self.all_contents[self.my_uri]
-        except KeyError:
-            raise NotEnoughSharesError()
+            data = self.all_contents[self.my_uri.to_string()]
+        except KeyError, le:
+            raise NotEnoughSharesError(le)
         return len(data)
     def read(self, consumer, offset=0, size=None):
         d = self.download_to_data()
@@ -184,7 +186,7 @@ class FakeMutableFileNode:
         return self.storage_index
 
     def check(self, monitor, verify=False):
-        r = CheckerResults(self.my_uri.to_string(), self.storage_index)
+        r = CheckerResults(self.my_uri, self.storage_index)
         is_bad = self.bad_shares.get(self.storage_index, None)
         data = {}
         data["count-shares-needed"] = 3
index d29e5da9e8fe61e340753a22448e5091186d7d9b..e2a4591af5713928354c03649b84f5e40c546463 100644 (file)
@@ -42,7 +42,7 @@ class Marker:
         return self.storage_index
 
     def check(self, monitor, verify=False):
-        r = CheckerResults("", None)
+        r = CheckerResults(uri.from_string(self.nodeuri), None)
         r.set_healthy(True)
         r.set_recoverable(True)
         return defer.succeed(r)
@@ -107,7 +107,7 @@ class Dirnode(unittest.TestCase,
         d = self.client.create_empty_dirnode()
         def _created(dn):
             u = make_mutable_file_uri()
-            d = dn.set_uri(u"child", u, {})
+            d = dn.set_uri(u"child", u.to_string(), {})
             d.addCallback(lambda res: dn.list())
             def _check1(children):
                 self.failUnless(u"child" in children)
@@ -239,7 +239,7 @@ class Dirnode(unittest.TestCase,
 
         d = self.client.create_empty_dirnode()
         def _created(rw_dn):
-            d2 = rw_dn.set_uri(u"child", fileuri)
+            d2 = rw_dn.set_uri(u"child", fileuri.to_string())
             d2.addCallback(lambda res: rw_dn)
             return d2
         d.addCallback(_created)
@@ -251,7 +251,7 @@ class Dirnode(unittest.TestCase,
             self.failUnless(ro_dn.is_mutable())
 
             self.shouldFail(dirnode.NotMutableError, "set_uri ro", None,
-                            ro_dn.set_uri, u"newchild", fileuri)
+                            ro_dn.set_uri, u"newchild", fileuri.to_string())
             self.shouldFail(dirnode.NotMutableError, "set_uri ro", None,
                             ro_dn.set_node, u"newchild", filenode)
             self.shouldFail(dirnode.NotMutableError, "set_nodes ro", None,
@@ -315,11 +315,11 @@ class Dirnode(unittest.TestCase,
             self.expected_manifest.append( ((u"child",) , m.get_uri()) )
             self.expected_verifycaps.add(ffu_v)
             self.expected_storage_indexes.add(base32.b2a(m.get_storage_index()))
-            d.addCallback(lambda res: n.set_uri(u"child", fake_file_uri))
+            d.addCallback(lambda res: n.set_uri(u"child", fake_file_uri.to_string()))
             d.addCallback(lambda res:
                           self.shouldFail(ExistingChildError, "set_uri-no",
                                           "child 'child' already exists",
-                                          n.set_uri, u"child", other_file_uri,
+                                          n.set_uri, u"child", other_file_uri.to_string(),
                                           overwrite=False))
             # /
             # /child = mutable
@@ -445,12 +445,12 @@ class Dirnode(unittest.TestCase,
 
             # set_uri + metadata
             # it should be possible to add a child without any metadata
-            d.addCallback(lambda res: n.set_uri(u"c2", fake_file_uri, {}))
+            d.addCallback(lambda res: n.set_uri(u"c2", fake_file_uri.to_string(), {}))
             d.addCallback(lambda res: n.get_metadata_for(u"c2"))
             d.addCallback(lambda metadata: self.failUnlessEqual(metadata, {}))
 
             # if we don't set any defaults, the child should get timestamps
-            d.addCallback(lambda res: n.set_uri(u"c3", fake_file_uri))
+            d.addCallback(lambda res: n.set_uri(u"c3", fake_file_uri.to_string()))
             d.addCallback(lambda res: n.get_metadata_for(u"c3"))
             d.addCallback(lambda metadata:
                           self.failUnlessEqual(sorted(metadata.keys()),
@@ -458,7 +458,7 @@ class Dirnode(unittest.TestCase,
 
             # or we can add specific metadata at set_uri() time, which
             # overrides the timestamps
-            d.addCallback(lambda res: n.set_uri(u"c4", fake_file_uri,
+            d.addCallback(lambda res: n.set_uri(u"c4", fake_file_uri.to_string(),
                                                 {"key": "value"}))
             d.addCallback(lambda res: n.get_metadata_for(u"c4"))
             d.addCallback(lambda metadata:
@@ -500,9 +500,9 @@ class Dirnode(unittest.TestCase,
             d.addCallback(lambda res: n.delete(u"d4"))
 
             # metadata through set_children()
-            d.addCallback(lambda res: n.set_children([ (u"e1", fake_file_uri),
-                                                   (u"e2", fake_file_uri, {}),
-                                                   (u"e3", fake_file_uri,
+            d.addCallback(lambda res: n.set_children([ (u"e1", fake_file_uri.to_string()),
+                                                   (u"e2", fake_file_uri.to_string(), {}),
+                                                   (u"e3", fake_file_uri.to_string(),
                                                     {"key": "value"}),
                                                    ]))
             d.addCallback(lambda res:
@@ -700,7 +700,7 @@ class Dirnode(unittest.TestCase,
 
             # now make sure that we honor overwrite=False
             d.addCallback(lambda res:
-                          self.subdir2.set_uri(u"newchild", other_file_uri))
+                          self.subdir2.set_uri(u"newchild", other_file_uri.to_string()))
 
             d.addCallback(lambda res:
                           self.shouldFail(ExistingChildError, "move_child_to-no",
index 4a91e36156322b5e919642e2f904ddb72bc814e1..2711a0e60a01602520914b05eb35a17a9c87d2b6 100644 (file)
@@ -9,6 +9,7 @@ from allmydata import interfaces, provisioning, uri, webish
 from allmydata.immutable import upload, download
 from allmydata.web import status, common
 from allmydata.util import fileutil, base32
+from allmydata.util.assertutil import precondition
 from allmydata.test.common import FakeDirectoryNode, FakeCHKFileNode, \
      FakeMutableFileNode, create_chk_filenode
 from allmydata.interfaces import IURI, INewDirectoryURI, \
@@ -56,6 +57,7 @@ class FakeClient(service.MultiService):
         return []
 
     def create_node_from_uri(self, auri):
+        precondition(isinstance(auri, str), auri)
         u = uri.from_string(auri)
         if (INewDirectoryURI.providedBy(u)
             or IReadonlyNewDirectoryURI.providedBy(u)):
@@ -2274,6 +2276,7 @@ class Web(WebMixin, testutil.StallMixin, unittest.TestCase):
         file_contents = "New file contents here\n"
         d = self.PUT("/uri", file_contents)
         def _check(uri):
+            assert isinstance(uri, str), uri
             self.failUnless(uri in FakeCHKFileNode.all_contents)
             self.failUnlessEqual(FakeCHKFileNode.all_contents[uri],
                                  file_contents)
@@ -2309,8 +2312,8 @@ class Web(WebMixin, testutil.StallMixin, unittest.TestCase):
         return d
 
         def _check(uri):
-            self.failUnless(uri in FakeCHKFileNode.all_contents)
-            self.failUnlessEqual(FakeCHKFileNode.all_contents[uri],
+            self.failUnless(uri.to_string() in FakeCHKFileNode.all_contents)
+            self.failUnlessEqual(FakeCHKFileNode.all_contents[uri.to_string()],
                                  file_contents)
             return self.GET("/uri/%s" % uri)
         d.addCallback(_check)