Support delete, also fix some encoding issues and similar bugs

When we delete, we don't actually delete, we just remove it from the name tree. To enable GC or undelete, we write a record of the deleted inodes in the 'X' keyspace
justinsb · Dec 20, 2013 · bbfb919 · bbfb919
1 parent 1f34ba7
commit bbfb919
Show file tree

Hide file tree

Showing 15 changed files with 740 additions and 24 deletions.
diff --git a/cloudata-files/src/main/java/com/cloudata/files/FilesModel.java b/cloudata-files/src/main/java/com/cloudata/files/FilesModel.java
diff --git a/cloudata-files/src/main/java/com/cloudata/files/fs/FsClient.java b/cloudata-files/src/main/java/com/cloudata/files/fs/FsClient.java
@@ -17,6 +17,7 @@
 import com.cloudata.clients.keyvalue.Modifier;
 import com.cloudata.files.FilesModel.ChunkData;
 import com.cloudata.files.FilesModel.ChunkData.Builder;
+import com.cloudata.files.FilesModel.DeletedData;
 import com.cloudata.files.FilesModel.InodeData;
 import com.cloudata.files.blobs.BlobCache.CacheFileHandle;
 import com.cloudata.files.blobs.BlobStore;
@@ -34,6 +35,7 @@ public class FsClient {
 
     private static final ByteString DIR_CHILDREN = ByteString.copyFrom(new byte[] { 'D' });
     private static final ByteString INODES = ByteString.copyFrom(new byte[] { 'I' });
+    private static final ByteString DELETED_INODES = ByteString.copyFrom(new byte[] { 'X' });
 
     private static final ByteString CHUNK_PREFIX = ByteString.copyFrom(new byte[] { 'C' });
 
@@ -100,6 +102,13 @@ private ByteString buildDirEntryKey(FsPath parent, ByteString name) {
         return key;
     }
 
+    private ByteString buildDirEntryKey(FsPath path) {
+        ByteString volumePrefix = path.getVolume().getPrefix();
+        ByteString key = volumePrefix.concat(DIR_CHILDREN).concat(ByteStrings.encode(path.getParent().getId()))
+                .concat(path.getNameBytes());
+        return key;
+    }
+
     public static class DirEntry {
         final FsPath parent;
         final ByteString name;
@@ -226,6 +235,8 @@ private void createNewEntry(FsPath parent, ByteString name, InodeData.Builder in
 
             inodeKey = buildInodeKey(parent.getVolume(), id);
             if (store.put(inodeKey, created.toByteString(), IfNotExists.INSTANCE)) {
+                log.debug("Wrote inode entry: {} = {}", id, created);
+
                 break;
             } else {
                 inode = InodeData.newBuilder(created);
@@ -301,4 +312,35 @@ public CacheFileHandle findChunk(ChunkData chunkData) throws IOException {
         return blobStore.find(key);
     }
 
+    public boolean delete(FsPath fsPath) throws IOException {
+        // Store a deletion record
+        // Allows for more efficient GC, and for undelete
+        // TODO: Support GC
+
+        long id = fsPath.getId();
+        DeletedData.Builder b = DeletedData.newBuilder();
+        b.setInode(id);
+        for (FsPath entry : fsPath.getPathComponentList()) {
+            b.addPath(entry.getNameBytes());
+        }
+        ByteString deletedValue = b.build().toByteString();
+
+        ByteString volumePrefix = fsPath.getVolume().getPrefix();
+
+        while (true) {
+            ByteString key = volumePrefix.concat(DELETED_INODES).concat(ByteStrings.encode(id))
+                    .concat(ByteStrings.encode(System.currentTimeMillis()));
+
+            if (store.put(key, deletedValue, IfNotExists.INSTANCE)) {
+                break;
+            }
+        }
+
+        // Unlink the entry from the name tree
+        {
+            ByteString key = buildDirEntryKey(fsPath);
+
+            return store.delete(key);
+        }
+    }
 }
diff --git a/cloudata-files/src/main/java/com/cloudata/files/fs/FsPath.java b/cloudata-files/src/main/java/com/cloudata/files/fs/FsPath.java
@@ -1,6 +1,9 @@
 package com.cloudata.files.fs;
 
+import java.util.List;
+
 import com.cloudata.files.FilesModel.InodeData;
+import com.google.common.collect.Lists;
 import com.google.protobuf.ByteString;
 
 public class FsPath extends Inode {
@@ -39,4 +42,26 @@ private void buildHref(StringBuilder sb) {
         }
     }
 
+    public FsPath getParent() {
+        return this.parent;
+    }
+
+    public List<FsPath> getPathComponentList() {
+        List<FsPath> components = Lists.newArrayList();
+        buildPathComponentList(components);
+        return components;
+    }
+
+    private void buildPathComponentList(List<FsPath> components) {
+        if (parent != null) {
+            parent.buildPathComponentList(components);
+        }
+        components.add(this);
+    }
+
+    @Override
+    public String toString() {
+        return "FsPath [href=" + getHref() + "]";
+    }
+
 }
diff --git a/cloudata-files/src/main/java/com/cloudata/files/fs/FsVolume.java b/cloudata-files/src/main/java/com/cloudata/files/fs/FsVolume.java
@@ -21,7 +21,7 @@ public static FsVolume fromHost(String host) {
         bucket = bucket.replace(":8080", "");
         bucket = bucket.replace("127.0.0.1", "local");
 
-        long key = 1;
+        long key = 2;
         return new FsVolume(key);
     }
 

diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/DeleteHandler.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/DeleteHandler.java
@@ -0,0 +1,34 @@
+package com.cloudata.files.webdav;
+
+import io.netty.handler.codec.http.HttpObject;
+import io.netty.handler.codec.http.HttpResponseStatus;
+
+import com.cloudata.files.fs.FsClient;
+import com.cloudata.files.fs.FsPath;
+
+public class DeleteHandler extends MethodHandler {
+
+    public DeleteHandler(WebdavRequestHandler requestHandler) {
+        super(requestHandler);
+    }
+
+    @Override
+    protected HttpObject doAction(FsPath fsPath) throws Exception {
+        WebdavRequest request = getRequest();
+        FsClient fsClient = getFsClient();
+
+        int depth = request.getDepth();
+
+        if (fsPath.isFolder()) {
+            if (depth != WebdavRequest.DEPTH_INFINITY) {
+                throw new WebdavResponseException(HttpResponseStatus.CONFLICT);
+            }
+        }
+
+        if (!fsClient.delete(fsPath)) {
+            throw new WebdavResponseException(HttpResponseStatus.CONFLICT);
+        }
+
+        return buildFullResponse(HttpResponseStatus.NO_CONTENT);
+    }
+}
diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/GetHandler.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/GetHandler.java
@@ -13,12 +13,17 @@
 import java.text.ParseException;
 import java.util.Date;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.cloudata.files.fs.FsFile;
 import com.cloudata.files.fs.FsPath;
 import com.google.common.base.Strings;
 import com.google.common.io.Closeables;
 
 public class GetHandler extends MethodHandler {
+    private static final Logger log = LoggerFactory.getLogger(GetHandler.class);
+
     public GetHandler(WebdavRequestHandler requestHandler) {
         super(requestHandler);
     }
@@ -37,6 +42,7 @@ public void close() {
 
     @Override
     public HttpObject doAction(FsPath fsPath) throws Exception {
+        log.debug("GET {}", fsPath);
         WebdavRequest request = getRequest();
 
         // TODO: Handle ETAG??
@@ -57,7 +63,7 @@ public HttpObject doAction(FsPath fsPath) throws Exception {
             long ifModifiedSinceDateSeconds = ifModifiedSinceDate.getTime() / 1000;
             long fileLastModifiedSeconds = fsPath.getModified() / 1000;
             if (ifModifiedSinceDateSeconds == fileLastModifiedSeconds) {
-                throw new WebdavResponseException(HttpResponseStatus.NOT_MODIFIED);
+                // throw new WebdavResponseException(HttpResponseStatus.NOT_MODIFIED);
             }
         }
 
@@ -107,6 +113,7 @@ public HttpObject doAction(FsPath fsPath) throws Exception {
         int httpCacheSeconds = 0;
         setDateAndCacheHeaders(fsPath, response, httpCacheSeconds);
 
+        log.debug("Sending response to GET: {} {}", fsPath.getHref(), response);
         write(response);
         if (content != null) {
             while (!content.isEndOfInput()) {

diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/LockHandler.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/LockHandler.java
@@ -39,7 +39,9 @@ protected HttpObject doAction(FsPath fsPath) throws Exception {
 
         log.debug("LockRequest: " + lockRequest);
 
-        CloudLockToken lockToken = lockService.tryLock(request.getUri());
+        String lockSubject = buildLockSubject(request);
+
+        CloudLockToken lockToken = lockService.tryLock(lockSubject);
         if (lockToken == null) {
             throw new WebdavResponseException(HttpResponseStatus.LOCKED);
         }

diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/MethodHandler.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/MethodHandler.java
@@ -129,15 +129,15 @@ private boolean checkIfHeader(String ifHeader) {
                         lockToken = lockToken.substring(LockHandler.LOCK_PREFIX.length());
                     }
 
-                    URI subjectUri;
+                    String lockSubject;
+
                     if (Strings.isNullOrEmpty(subjectUrl)) {
-                        subjectUri = URI.create(getRequest().getUri());
+                        lockSubject = buildLockSubject(getRequest());
                     } else {
-                        subjectUri = URI.create(subjectUrl);
+                        lockSubject = buildLockSubject(subjectUrl);
                     }
-                    String path = subjectUri.getPath();
 
-                    if (getLockService().findLock(path, lockToken) == null) {
+                    if (getLockService().findLock(lockSubject, lockToken) == null) {
                         return false;
                     }
                 } else {
@@ -274,7 +274,7 @@ public FsPath call() throws Exception {
 
                     pathTokens = Lists.newArrayList();
                     for (String token : Splitter.on('/').split(path)) {
-                        pathTokens.add(token);
+                        pathTokens.add(Urls.decodeComponent(token));
                     }
                 }
 
@@ -291,4 +291,14 @@ protected boolean shouldResolveParent() {
     private ListeningExecutorService getExecutor() {
         return requestHandler.getExecutor();
     }
+
+    protected String buildLockSubject(WebdavRequest request) {
+        String uri = request.getUri();
+        return buildLockSubject(uri);
+    }
+
+    protected String buildLockSubject(String uriString) {
+        URI uri = URI.create(uriString);
+        return uri.getPath();
+    }
 }
diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/MkdirHandler.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/MkdirHandler.java
@@ -34,7 +34,7 @@ protected HttpObject doAction(FsPath parentPath) throws Exception {
         }
 
         String path = request.getUri();
-        String name = Urls.getLastPathComponent(path);
+        String name = Urls.getLastPathComponent(path, true);
 
         InodeData.Builder inode = InodeData.newBuilder();
 

diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/PutHandler.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/PutHandler.java
@@ -4,6 +4,9 @@
 import io.netty.handler.codec.http.HttpObject;
 import io.netty.handler.codec.http.HttpResponseStatus;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.cloudata.files.FilesModel.InodeData;
 import com.cloudata.files.fs.FsPath;
 import com.cloudata.files.webdav.chunks.ChunkAccumulator;
@@ -13,6 +16,8 @@
 
 public class PutHandler extends MethodHandler {
 
+    private static final Logger log = LoggerFactory.getLogger(PutHandler.class);
+
     public PutHandler(WebdavRequestHandler requestHandler) {
         super(requestHandler);
     }
@@ -29,7 +34,9 @@ public HttpObject doAction(FsPath parentPath) throws Exception {
         }
 
         String path = getUri();
-        String newName = Urls.getLastPathComponent(path);
+        String newName = Urls.getLastPathComponent(path, true);
+
+        log.debug("PUT {} {}", parentPath, newName);
 
         // TODO: Always overwrite??
         boolean overwrite = true;

diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/UnlockHandler.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/UnlockHandler.java
@@ -37,8 +37,9 @@ protected HttpObject doAction(FsPath fsPath) throws Exception {
         if (lockToken.endsWith(">")) {
             lockToken = lockToken.substring(0, lockToken.length() - 1);
         }
+        String lockSubject = buildLockSubject(request);
 
-        boolean unlocked = lockService.unlock(request.getUri(), lockToken);
+        boolean unlocked = lockService.unlock(lockSubject, lockToken);
         if (!unlocked) {
             throw new IllegalArgumentException("Lock not found");
         }

diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/Urls.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/Urls.java
@@ -1,5 +1,10 @@
 package com.cloudata.files.webdav;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
+
+import com.google.common.base.Throwables;
+
 public class Urls {
 
     public static String trimLastSlash(String path) {
@@ -25,14 +30,26 @@ public static String getParentPath(String path) {
         return path.substring(0, lastSlashIndex + 1);
     }
 
-    public static String getLastPathComponent(String path) {
+    public static String getLastPathComponent(String path, boolean decode) {
         path = trimLastSlash(path);
 
         int lastSlashIndex = path.lastIndexOf('/');
         if (lastSlashIndex == -1) {
             return null;
         }
-        return path.substring(lastSlashIndex + 1);
+        String component = path.substring(lastSlashIndex + 1);
+        if (decode) {
+            component = decodeComponent(component);
+        }
+        return component;
+    }
+
+    public static String decodeComponent(String component) {
+        try {
+            return URLDecoder.decode(component, "UTF-8");
+        } catch (UnsupportedEncodingException e) {
+            throw Throwables.propagate(e);
+        }
     }
 
 }
diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/WebdavRequestHandler.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/WebdavRequestHandler.java
@@ -93,7 +93,7 @@ public ListenableFuture<HttpObject> processRequest() throws Exception {
         } else if (request.getMethod() == HttpMethod.PUT) {
             handler = new PutHandler(this);
         } else if (request.getMethod() == HttpMethod.DELETE) {
-            throw new UnsupportedOperationException();
+            handler = new DeleteHandler(this);
         } else if (request.getMethod().name().equals("PROPPATCH")) {
             throw new UnsupportedOperationException();
         } else {

diff --git a/cloudata-files/src/main/java/com/cloudata/files/webdav/chunks/SimpleChunkAccumulator.java b/cloudata-files/src/main/java/com/cloudata/files/webdav/chunks/SimpleChunkAccumulator.java
@@ -44,7 +44,7 @@ public synchronized void append(ByteBuf content) throws IOException {
             }
         } else {
             content.readBytes(os, length);
-            totalSize += content.readableBytes();
+            totalSize += length;
         }
     }
 
@@ -110,16 +110,24 @@ public void close() throws IOException {
 
     public ByteSource getByteSource() throws IOException {
         if (buffers != null) {
+            assert file == null;
+
             List<ByteSource> byteSources = Lists.newArrayList();
             for (ByteBuf buf : buffers) {
                 byteSources.add(new ByteBufByteSource(buf));
             }
             return ByteSource.concat(byteSources);
         } else if (file != null) {
+            assert buffers == null;
+
             if (os != null) {
                 os.flush();
             }
 
+            if (file.length() != totalSize) {
+                throw new IllegalStateException();
+            }
+
             return Files.asByteSource(file);
         } else {
             return ByteSource.empty();

diff --git a/cloudata-files/src/main/proto/FilesModel.proto b/cloudata-files/src/main/proto/FilesModel.proto
@@ -5,6 +5,11 @@ message ChunkData {
 	optional bytes hash = 2;
 }
 
+message DeletedData {
+	optional uint64 inode = 1;
+	repeated bytes path = 2;
+}
+
 message InodeData {
 	optional uint64 length = 1;
 	optional uint64 inode = 2;