mirror of
https://github.com/tahoe-lafs/tahoe-lafs.git
synced 2025-04-08 19:34:18 +00:00
try to tidy up uri-as-string vs. uri-as-object
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.
This commit is contained in:
parent
7b285ebcb1
commit
471e1f1b9b
@ -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 = []
|
||||
|
@ -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)
|
||||
|
@ -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,
|
||||
})
|
||||
|
@ -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,
|
||||
|
@ -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?
|
||||
|
@ -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
|
||||
|
@ -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",
|
||||
|
@ -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)
|
||||
|
Loading…
x
Reference in New Issue
Block a user