From: Brian Warner Date: Wed, 24 Jan 2007 22:10:53 +0000 (-0700) Subject: filetree: make delete() work X-Git-Tag: tahoe_v0.1.0-0-UNSTABLE~329 X-Git-Url: https://git.rkrishnan.org/something?a=commitdiff_plain;h=bbf188d2c6aa732e823da07f609200df1f96599f;p=tahoe-lafs%2Ftahoe-lafs.git filetree: make delete() work --- diff --git a/src/allmydata/filetree/directory.py b/src/allmydata/filetree/directory.py index 81287718..02a9397b 100644 --- a/src/allmydata/filetree/directory.py +++ b/src/allmydata/filetree/directory.py @@ -207,6 +207,25 @@ class _DirectorySubTree(object): # now we can finally add the new node node.add(child_name, new_node) + def delete_node_at_path(self, path): + assert len(path) > 0 + child_name = path[-1] + + # first step: get the parent directory + node = self.root + for subdir_name in path[:-1]: + subdir_node = node.get(subdir_name) # may raise NoSuchChildError + node = subdir_node + + # 'node' is now pointing at the parent directory. Let's make sure the + # path they want to delete actually exists. We don't really care what + # the child *is*, just that it exists. + node.get(child_name) # may raise NoSuchChildError + + # now delete it + # TODO: How do we free the subtree that was just orphaned? + node.delete(child_name) + class LocalFileSubTreeNode(BaseDataNode): prefix = "LocalFileDirectory" diff --git a/src/allmydata/filetree/interfaces.py b/src/allmydata/filetree/interfaces.py index 76465bb1..700c2046 100644 --- a/src/allmydata/filetree/interfaces.py +++ b/src/allmydata/filetree/interfaces.py @@ -166,6 +166,12 @@ class ISubTree(Interface): run synchronously, and returns None. """ + def delete_node_at_path(path): + """Delete the node at the the given path. + + This must run synchronously, and returns None. + """ + def serialize_subtree_to_file(f): """Create a string which describes my structure and write it to the given filehandle (using only .write()). This string should be @@ -279,16 +285,18 @@ class IVirtualDrive(Interface): is complete. """ - def upload_now(path, uploadable): - """Upload a file to the given path. The path must not already exist. + def upload_data(path, data): + """Upload a string to the given path. The path must not already exist. - path[:-1] must refer to a writable DIRECTORY node. 'uploadable' must - implement IUploadable. This returns a Deferred that fires (with - 'uploadable') when the upload is complete. Do not use the workqueue. + path[:-1] must refer to a writable DIRECTORY node. + + This uses the workqueue, and returns None. """ - def upload_later(path, filename): - """Upload a file from disk to the given path. Use the workqueue. + def upload(path, filename): + """Upload a file from disk to the given path. + + This uses the workqueue, and returns None. """ def delete(path): diff --git a/src/allmydata/filetree/vdrive.py b/src/allmydata/filetree/vdrive.py index a8494e15..438b4ac5 100644 --- a/src/allmydata/filetree/vdrive.py +++ b/src/allmydata/filetree/vdrive.py @@ -74,6 +74,7 @@ class SubTreeMaker(object): class VirtualDrive(object): implements(IVirtualDrive) + debug = False def __init__(self, workqueue, downloader, uploader, root_node): assert IWorkQueue(workqueue) @@ -242,6 +243,8 @@ class VirtualDrive(object): # as necessary def _got_subtrees(subtrees, new_node_boxname): for (subtree, subpath) in reversed(subtrees): + if self.debug: + print "SUBTREE", subtree, subpath assert subtree.is_mutable() must_update = subtree.mutation_modifies_parent() subtree_node = subtree.create_node_now() @@ -263,6 +266,11 @@ class VirtualDrive(object): d.addCallback(_got_subtrees, new_node_boxname) return d + def deletepath(self, path): + if self.debug: + print "DELETEPATH(%s)" % (path,) + return self.addpath(path, None) + def modify_subtree(self, subtree_node, localpath, new_node, new_subtree_boxname=None): # TODO: I'm lying here, we don't know who the parent is, so we can't @@ -278,7 +286,10 @@ class VirtualDrive(object): parent_is_mutable) def _got_subtree(subtree): assert subtree.is_mutable() - subtree.put_node_at_path(localpath, new_node) + if new_node: + subtree.put_node_at_path(localpath, new_node) + else: + subtree.delete_node_at_path(localpath) return subtree.update_now(self._uploader) d.addCallback(_got_subtree) if new_subtree_boxname: @@ -312,26 +323,18 @@ class VirtualDrive(object): target = download.Data() return self.download(path, target) - def upload_now(self, path, uploadable): + def upload_data(self, path, data): assert isinstance(path, list) - # note: the first few steps of this do not use the workqueue, but I - # think things should remain consistent anyways. If the node is shut - # down before the file has finished uploading, then we forget all - # abou the file. - uploadable = IUploadable(uploadable) - d = self._child_should_not_exist(path) - # then we upload the file - d.addCallback(lambda ignored: self._uploader.upload(uploadable)) - def _uploaded(uri): - assert isinstance(uri, str) - new_node = file.CHKFileNode().new(uri) - boxname = self.workqueue.create_boxname(new_node) - self.workqueue.add_addpath(boxname, path) - self.workqueue.add_delete_box(boxname) - d.addCallback(_uploaded) - return d + f, tempfilename = self.workqueue.create_tempfile() + f.write(data) + f.close() + boxname = self.workqueue.create_boxname() + self.workqueue.add_upload_chk(tempfilename, boxname) + self.workqueue.add_addpath(boxname, path) + self.workqueue.add_delete_box(boxname) + self.workqueue.add_delete_tempfile(tempfilename) - def upload_later(self, path, filename): + def upload(self, path, filename): assert isinstance(path, list) filename = os.path.abspath(filename) boxname = self.workqueue.create_boxname() @@ -341,21 +344,7 @@ class VirtualDrive(object): def delete(self, path): assert isinstance(path, list) - parent_path = path[:-1] - orphan_path = path[-1] - d = self._get_closest_node_and_prepath(parent_path) - def _got_parent((prepath, node, remaining_path)): - assert not remaining_path - node.delete(orphan_path) - # now serialize and upload - subtree = node.get_subtree() - boxname = subtree.update(self.workqueue) - if boxname: - self.workqueue.add_addpath(boxname, prepath) - self.workqueue.add_delete_box(boxname) - return self - d.addCallback(_got_parent) - return d + self.workqueue.add_deletepath(path) def add_node(self, path, node): assert isinstance(path, list) diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index d1cd48ea..024d7ff5 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -224,6 +224,15 @@ class IUploader(Interface): def upload_ssk(write_capability, new_version, uploadable): pass # TODO + def upload_data(data): + """Like upload(), but accepts a string.""" + + def upload_filename(filename): + """Like upload(), but accepts an absolute pathname.""" + + def upload_filehandle(filehane): + """Like upload(), but accepts an open filehandle.""" + class IWorkQueue(Interface): """Each filetable root is associated a work queue, which is persisted on @@ -315,6 +324,13 @@ class IWorkQueue(Interface): steps to be added to the workqueue. """ + def add_deletepath(path): + """When executed, finds the subtree that contains the node at 'path' + and modifies it (and any necessary parent subtrees) to delete that + path. This will probably cause one or more 'add_modify_subtree' or + 'add_modify_redirection' steps to be added to the workqueue. + """ + def add_modify_subtree(subtree_node, localpath, new_node_boxname, new_subtree_boxname=None): """When executed, this step retrieves the subtree specified by @@ -322,7 +338,8 @@ class IWorkQueue(Interface): then modifies it such that a subtree-relative 'localpath' points to the new node. It then serializes the subtree in its new form, and optionally puts a node that describes the new subtree in - 'new_node_boxname'. + 'new_node_boxname'. If 'new_node_boxname' is None, this deletes the + given path. The idea is that 'subtree_node' will refer a CHKDirectorySubTree, and 'new_node_boxname' will contain the CHKFileNode that points to a diff --git a/src/allmydata/test/test_filetree_new.py b/src/allmydata/test/test_filetree_new.py index 7c6d05ee..4d478112 100644 --- a/src/allmydata/test/test_filetree_new.py +++ b/src/allmydata/test/test_filetree_new.py @@ -323,10 +323,11 @@ from allmydata.filetree.interfaces import (ISubTree, INode, IDirectoryNode, IFileNode, NoSuchDirectoryError, NoSuchChildError) from allmydata.filetree.file import CHKFileNode +from allmydata import upload from allmydata.interfaces import IDownloader from allmydata.util import bencode -class InPairs(unittest.TestCase): +class Utils(unittest.TestCase): def test_in_pairs(self): l = range(8) pairs = list(directory.in_pairs(l)) @@ -338,19 +339,28 @@ class FakeMesh(object): def __init__(self): self.files = {} - def upload_filename(self, filename): + + def upload(self, uploadable): uri = "stub-uri-%d" % len(self.files) if self.debug: - print "FakeMesh.upload_filename(%s) -> %s" % (filename, uri) - data = open(filename,"r").read() + print "FakeMesh.upload -> %s" % uri + assert upload.IUploadable.providedBy(uploadable) + f = uploadable.get_filehandle() + data = f.read() + uploadable.close_filehandle(f) self.files[uri] = data return defer.succeed(uri) + + def upload_filename(self, filename): + if self.debug: + print "FakeMesh.upload_filename(%s)" % filename + return self.upload(upload.FileName(filename)) + def upload_data(self, data): - uri = "stub-uri-%d" % len(self.files) if self.debug: - print "FakeMesh.upload_data(%s) -> %s" % (data, uri) - self.files[uri] = data - return defer.succeed(uri) + print "FakeMesh.upload_data(%s)" % data + return self.upload(upload.Data(data)) + def download(self, uri, target): if self.debug: print "FakeMesh.download(%s)" % uri @@ -363,7 +373,9 @@ class FakeMesh(object): class VDrive(unittest.TestCase): def makeVirtualDrive(self, basedir, root_node=None, mesh=None): - wq = workqueue.WorkQueue(os.path.join(basedir, "1.workqueue")) + wq = workqueue.WorkQueue(os.path.join("test_filetree", + "VDrive", + basedir, "1.workqueue")) if mesh: assert IUploader.providedBy(mesh) assert IDownloader.providedBy(mesh) @@ -557,7 +569,7 @@ class VDrive(unittest.TestCase): f.write(DATA) f.close() - rc = v.upload_later(["a","b","upload1"], filename) + rc = v.upload(["a","b","upload1"], filename) self.failUnlessIdentical(rc, None) d = v.workqueue.flush() @@ -579,16 +591,16 @@ class VDrive(unittest.TestCase): def testCHKDirUpload(self): DATA = "here is some data\n" - d = defer.maybeDeferred(self.makeCHKTree, "upload") + filename = "upload1" + f = open(filename, "w") + f.write(DATA) + f.close() + + d = defer.maybeDeferred(self.makeCHKTree, "chk-upload") def _made(v): self.v = v - filename = "upload1" - f = open(filename, "w") - f.write(DATA) - f.close() - - rc = v.upload_later(["a","b","upload1"], filename) + rc = v.upload(["a","b","upload1"], filename) self.failUnlessIdentical(rc, None) return v.workqueue.flush() @@ -609,3 +621,47 @@ class VDrive(unittest.TestCase): return d + def testCHKDirDelete(self): + DATA = "here is some data\n" + filename = "upload1" + f = open(filename, "w") + f.write(DATA) + f.close() + + d = defer.maybeDeferred(self.makeCHKTree, "chk-delete") + def _made(v): + self.v = v + d.addCallback(_made) + + d.addCallback(lambda r: + self.v.upload(["a","b","upload1"], filename)) + d.addCallback(lambda r: + self.v.upload_data(["a","b","upload2"], DATA)) + d.addCallback(lambda r: + self.v.upload(["a","c","upload3"], filename)) + d.addCallback(lambda r: + self.v.workqueue.flush()) + + d.addCallback(lambda r: self.v.list([])) + d.addCallback(lambda contents: + self.failUnlessListsAreEqual(contents.keys(), ["a"])) + d.addCallback(lambda r: self.v.list(["a"])) + d.addCallback(lambda contents: + self.failUnlessListsAreEqual(contents.keys(), ["b","c"])) + d.addCallback(lambda r: self.v.list(["a","b"])) + d.addCallback(lambda contents: + self.failUnlessListsAreEqual(contents.keys(), + ["upload1", "upload2"])) + #d.addCallback(lambda r: self.v.download_as_data(["a","b","upload1"])) + #d.addCallback(self.failUnlessEqual, DATA) + + # now delete it + d.addCallback(lambda r: self.v.delete(["a","b","upload2"])) + d.addCallback(lambda r: self.v.workqueue.flush()) + d.addCallback(lambda r: self.v.list(["a","b"])) + d.addCallback(lambda contents: + self.failUnlessListsAreEqual(contents.keys(), + ["upload1"])) + + + return d diff --git a/src/allmydata/workqueue.py b/src/allmydata/workqueue.py index 3b037c59..1ad47a9e 100644 --- a/src/allmydata/workqueue.py +++ b/src/allmydata/workqueue.py @@ -177,6 +177,12 @@ class WorkQueue(object): lines.extend(path) self._create_step_first(lines) + def add_deletepath(self, path): + assert isinstance(path, (list, tuple)) + lines = ["deletepath"] + lines.extend(path) + self._create_step_first(lines) + def add_modify_subtree(self, subtree_node, localpath, new_node_boxname, new_subtree_boxname=None): assert isinstance(localpath, (list, tuple)) @@ -184,6 +190,8 @@ class WorkQueue(object): self.add_delete_box(box1) # TODO: it would probably be easier if steps were represented in # directories, with a separate file for each argument + if new_node_boxname is None: + new_node_boxname = "" if new_subtree_boxname is None: new_subtree_boxname = "" lines = ["modify_subtree", @@ -323,6 +331,13 @@ class WorkQueue(object): print "STEP_ADDPATH(%s -> %s)" % (boxname, "/".join(path)) path = list(path) return self.vdrive.addpath(path, boxname) + + def step_deletepath(self, *path): + if self.debug: + print "STEP_DELETEPATH(%s)" % "/".join(path) + path = list(path) + return self.vdrive.deletepath(path) + def step_modify_subtree(self, subtree_node_boxname, new_node_boxname, new_subtree_boxname, *localpath): # the weird order of arguments is a consequence of the fact that @@ -330,7 +345,9 @@ class WorkQueue(object): if not new_subtree_boxname: new_subtree_boxname = None subtree_node = self.read_from_box(subtree_node_boxname) - new_node = self.read_from_box(new_node_boxname) + new_node = None + if new_node_boxname: + new_node = self.read_from_box(new_node_boxname) localpath = list(localpath) return self.vdrive.modify_subtree(subtree_node, localpath, new_node, new_subtree_boxname)