Implement filesystem storage in rust (#724)

* Implement filesystem storage in rust

* Fix circleci

* stylefixes

rust/Cargo.lock

@@ -236,6 +236,16 @@ dependencies = [
  "libc 0.2.35 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "rand"
+version = "0.4.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "fuchsia-zircon 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.35 (registry+https://github.com/rust-lang/crates.io-index)",
+ "winapi 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "rayon"
 version = "0.8.2"
@@ -418,6 +428,14 @@ name = "untrusted"
 version = "0.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "uuid"
+version = "0.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "vdirsyncer-rustext"
 version = "0.1.0"
@@ -425,8 +443,11 @@ dependencies = [
  "atomicwrites 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "cbindgen 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.35 (registry+https://github.com/rust-lang/crates.io-index)",
+ "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "ring 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "shippai 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "uuid 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "vobject 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -509,6 +530,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c51a3322e4bca9d212ad9a158a02abc6934d005490c054a2778df73a70aa0a30"
 "checksum quote 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)" = "7a6e920b65c65f10b2ae65c831a81a073a89edd28c7cce89475bff467ab4167a"
 "checksum rand 0.3.20 (registry+https://github.com/rust-lang/crates.io-index)" = "512870020642bb8c221bf68baa1b2573da814f6ccfe5c9699b1c303047abe9b1"
+"checksum rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "eba5f8cb59cc50ed56be8880a5c7b496bfd9bd26394e176bc67884094145c2c5"
 "checksum rayon 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)" = "b614fe08b6665cb9a231d07ac1364b0ef3cb3698f1239ee0c4c3a88a524f54c8"
 "checksum rayon-core 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e64b609139d83da75902f88fd6c01820046840a18471e4dfcd5ac7c0f46bea53"
 "checksum redox_syscall 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)" = "0d92eecebad22b767915e4d529f89f28ee96dbbf5a4810d2b844373f136417fd"
@@ -532,6 +554,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum unicode-width 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "bf3a113775714a22dcb774d8ea3655c53a32debae63a063acc00a91cc586245f"
 "checksum unicode-xid 0.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "8c1f860d7d29cf02cb2f3f359fd35991af3d30bac52c57d265a3c461074cb4dc"
 "checksum untrusted 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f392d7819dbe58833e26872f5f6f0d68b7bbbe90fc3667e98731c4a15ad9a7ae"
+"checksum uuid 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "990fb49481275abe3c8e2a91339c009cd6146d9f38fc3413e4163d892cbaffbb"
 "checksum vec_map 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "887b5b631c2ad01628bbbaa7dd4c869f80d3186688f8d0b6f58774fbe324988c"
 "checksum vobject 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6041995691036270fabeb41975ca858f3b5113b82eea19a4f276bfb8b32e9ae4"
 "checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d"

rust/Cargo.toml

@@ -13,6 +13,9 @@ ring = "0.12.1"
 failure = "0.1"
 shippai = "0.1.1"
 atomicwrites = "0.1.4"
+uuid = { version = "0.6", features = ["v4"] }
+libc = "0.2"
+log = "0.4"
 
 [build-dependencies]
 cbindgen = "0.4"

rust/src/lib.rs

@@ -3,8 +3,12 @@ extern crate atomicwrites;
 extern crate failure;
 #[macro_use]
 extern crate shippai;
+extern crate libc;
 extern crate ring;
+extern crate uuid;
 extern crate vobject;
+#[macro_use]
+extern crate log;
 
 mod item;
 mod storage;

rust/src/storage/exports.rs

@@ -5,6 +5,7 @@ use errors::*;
 use item::Item;
 use super::Storage;
 pub use super::singlefile::exports::*;
+pub use super::filesystem::exports::*;
 
 #[no_mangle]
 pub unsafe extern "C" fn vdirsyncer_storage_free(storage: *mut Box<Storage>) {

rust/src/storage/filesystem.rs

@@ -0,0 +1,220 @@
+use std::path::{Path, PathBuf};
+use std::fs;
+use std::io;
+use std::io::{Read, Write};
+use std::os::unix::fs::MetadataExt;
+use std::process::Command;
+use super::Storage;
+use errors::*;
+use libc;
+use failure;
+
+use super::utils;
+
+use item::Item;
+
+use atomicwrites::{AllowOverwrite, AtomicFile, DisallowOverwrite};
+
+pub struct FilesystemStorage {
+    path: PathBuf,
+    fileext: String,
+    post_hook: Option<String>,
+}
+
+impl FilesystemStorage {
+    pub fn new<P: AsRef<Path>>(path: P, fileext: &str, post_hook: Option<String>) -> Self {
+        FilesystemStorage {
+            path: path.as_ref().to_owned(),
+            fileext: fileext.into(),
+            post_hook,
+        }
+    }
+
+    fn get_href(&self, ident: Option<&str>) -> String {
+        let href_base = match ident {
+            Some(x) => utils::generate_href(x),
+            None => utils::random_href(),
+        };
+        format!("{}{}", href_base, self.fileext)
+    }
+
+    fn get_filepath(&self, href: &str) -> PathBuf {
+        self.path.join(href)
+    }
+
+    fn run_post_hook<S: AsRef<::std::ffi::OsStr>>(&self, fpath: S) {
+        if let Some(ref cmd) = self.post_hook {
+            let status = match Command::new(cmd).arg(fpath).status() {
+                Ok(x) => x,
+                Err(e) => {
+                    warn!("Failed to run external hook: {}", e);
+                    return;
+                }
+            };
+
+            if !status.success() {
+                if let Some(code) = status.code() {
+                    warn!("External hook exited with error code {}.", code);
+                } else {
+                    warn!("External hook was killed.");
+                }
+            }
+        }
+    }
+}
+
+#[inline]
+fn handle_io_error(href: &str, e: io::Error) -> failure::Error {
+    match e.kind() {
+        io::ErrorKind::NotFound => ItemNotFound {
+            href: href.to_owned(),
+        }.into(),
+        io::ErrorKind::AlreadyExists => ItemAlreadyExisting {
+            href: href.to_owned(),
+        }.into(),
+        _ => e.into(),
+    }
+}
+
+pub mod exports {
+    use super::*;
+    use std::ffi::CStr;
+    use std::os::raw::c_char;
+
+    #[no_mangle]
+    pub unsafe extern "C" fn vdirsyncer_init_filesystem(
+        path: *const c_char,
+        fileext: *const c_char,
+        post_hook: *const c_char,
+    ) -> *mut Box<Storage> {
+        let path_c = CStr::from_ptr(path);
+        let fileext_c = CStr::from_ptr(fileext);
+        let post_hook_c = CStr::from_ptr(post_hook);
+        let post_hook_str = post_hook_c.to_str().unwrap();
+
+        Box::into_raw(Box::new(Box::new(FilesystemStorage::new(
+            path_c.to_str().unwrap(),
+            fileext_c.to_str().unwrap(),
+            if post_hook_str.is_empty() {
+                None
+            } else {
+                Some(post_hook_str.to_owned())
+            },
+        ))))
+    }
+}
+
+#[inline]
+fn etag_from_file(metadata: &fs::Metadata) -> String {
+    format!(
+        "{}.{};{}",
+        metadata.mtime(),
+        metadata.mtime_nsec(),
+        metadata.ino()
+    )
+}
+
+impl Storage for FilesystemStorage {
+    fn list<'a>(&'a mut self) -> Fallible<Box<Iterator<Item = (String, String)> + 'a>> {
+        let mut rv: Vec<(String, String)> = vec![];
+
+        for entry_res in fs::read_dir(&self.path)? {
+            let entry = entry_res?;
+            let metadata = entry.metadata()?;
+
+            if !metadata.is_file() {
+                continue;
+            }
+
+            let fname: String = match entry.file_name().into_string() {
+                Ok(x) => x,
+                Err(_) => continue,
+            };
+
+            if !fname.ends_with(&self.fileext) {
+                continue;
+            }
+
+            rv.push((fname, etag_from_file(&metadata)));
+        }
+
+        Ok(Box::new(rv.into_iter()))
+    }
+
+    fn get(&mut self, href: &str) -> Fallible<(Item, String)> {
+        let fpath = self.get_filepath(href);
+        let mut f = match fs::File::open(fpath) {
+            Ok(x) => x,
+            Err(e) => Err(handle_io_error(href, e))?,
+        };
+
+        let mut s = String::new();
+        f.read_to_string(&mut s)?;
+        Ok((Item::from_raw(s), etag_from_file(&f.metadata()?)))
+    }
+
+    fn upload(&mut self, item: Item) -> Fallible<(String, String)> {
+        #[inline]
+        fn inner(s: &mut FilesystemStorage, item: &Item, href: &str) -> io::Result<String> {
+            let filepath = s.get_filepath(href);
+            let af = AtomicFile::new(&filepath, DisallowOverwrite);
+            let content = item.get_raw();
+            af.write(|f| f.write_all(content.as_bytes()))?;
+            let new_etag = etag_from_file(&fs::metadata(&filepath)?);
+            s.run_post_hook(filepath);
+            Ok(new_etag)
+        }
+
+        let ident = item.get_ident()?;
+        let mut href = self.get_href(Some(&ident));
+        let etag = match inner(self, &item, &href) {
+            Ok(x) => x,
+            Err(ref e) if e.raw_os_error() == Some(libc::ENAMETOOLONG) => {
+                href = self.get_href(None);
+                match inner(self, &item, &href) {
+                    Ok(x) => x,
+                    Err(e) => Err(handle_io_error(&href, e))?,
+                }
+            }
+            Err(e) => Err(handle_io_error(&href, e))?,
+        };
+
+        Ok((href, etag))
+    }
+
+    fn update(&mut self, href: &str, item: Item, etag: &str) -> Fallible<String> {
+        let filepath = self.get_filepath(href);
+        let metadata = match fs::metadata(&filepath) {
+            Ok(x) => x,
+            Err(e) => Err(handle_io_error(href, e))?,
+        };
+        let actual_etag = etag_from_file(&metadata);
+        if actual_etag != etag {
+            Err(WrongEtag {
+                href: href.to_owned(),
+            })?;
+        }
+
+        let af = AtomicFile::new(&filepath, AllowOverwrite);
+        let content = item.get_raw();
+        af.write(|f| f.write_all(content.as_bytes()))?;
+        let new_etag = etag_from_file(&fs::metadata(filepath)?);
+        Ok(new_etag)
+    }
+
+    fn delete(&mut self, href: &str, etag: &str) -> Fallible<()> {
+        let filepath = self.get_filepath(href);
+        let metadata = match fs::metadata(&filepath) {
+            Ok(x) => x,
+            Err(e) => Err(handle_io_error(href, e))?,
+        };
+        let actual_etag = etag_from_file(&metadata);
+        if actual_etag != etag {
+            Err(WrongEtag {
+                href: href.to_owned(),
+            })?;
+        }
+        fs::remove_file(filepath)?;
+        Ok(())
+    }
+}

rust/src/storage/mod.rs

@@ -1,5 +1,7 @@
 pub mod singlefile;
 pub mod exports;
+pub mod filesystem;
+mod utils;
 use errors::Fallible;
 use item::Item;
 

rust/src/storage/utils.rs

@@ -0,0 +1,24 @@
+use uuid::Uuid;
+
+fn is_href_safe(ident: &str) -> bool {
+    for c in ident.chars() {
+        match c {
+            '_' | '.' | '-' | '+' => (),
+            _ if c.is_alphanumeric() => (),
+            _ => return false,
+        }
+    }
+    true
+}
+
+pub fn generate_href(ident: &str) -> String {
+    if is_href_safe(ident) {
+        ident.to_owned()
+    } else {
+        random_href()
+    }
+}
+
+pub fn random_href() -> String {
+    format!("{}", Uuid::new_v4())
+}

rust/vdirsyncer_rustext.h

@@ -50,6 +50,10 @@ const char *vdirsyncer_get_raw(Item *c);
 
 const char *vdirsyncer_get_uid(Item *c);
 
+Box_Storage *vdirsyncer_init_filesystem(const char *path,
+                                        const char *fileext,
+                                        const char *post_hook);
+
 Box_Storage *vdirsyncer_init_singlefile(const char *path);
 
 Item *vdirsyncer_item_from_raw(const char *s);

scripts/circleci-install.sh

@@ -1,11 +1,11 @@
-echo "export PATH=$HOME/.cargo/bin/:$PATH" >> $BASH_ENV
+echo "export PATH=$HOME/.cargo/bin/:$HOME/.local/bin/:$PATH" >> $BASH_ENV
 . $BASH_ENV
 
 make install-rust
 sudo apt-get install -y cmake
 
 pip install --user virtualenv
-~/.local/bin/virtualenv ~/env
+virtualenv ~/env
 
 echo ". ~/env/bin/activate" >> $BASH_ENV
 . $BASH_ENV

tests/storage/test_filesystem.py

@@ -1,7 +1,5 @@
 # -*- coding: utf-8 -*-
 
-import subprocess
-
 import pytest
 
 from vdirsyncer.storage.filesystem import FilesystemStorage
@@ -29,17 +27,6 @@ class TestFilesystemStorage(StorageTests):
             f.write('stub')
             self.storage_class(str(tmpdir) + '/hue', '.txt')
 
-    def test_broken_data(self, tmpdir):
-        s = self.storage_class(str(tmpdir), '.txt')
-
-        class BrokenItem(object):
-            raw = u'Ц, Ш, Л, ж, Д, З, Ю'.encode('utf-8')
-            uid = 'jeezus'
-            ident = uid
-        with pytest.raises(TypeError):
-            s.upload(BrokenItem)
-        assert not tmpdir.listdir()
-
     def test_ident_with_slash(self, tmpdir):
         s = self.storage_class(str(tmpdir), '.txt')
         s.upload(format_item('a/b/c'))
@@ -52,31 +39,10 @@ class TestFilesystemStorage(StorageTests):
         href, etag = s.upload(item)
         assert item.uid not in href
 
-    def test_post_hook_inactive(self, tmpdir, monkeypatch):
-
-        def check_call_mock(*args, **kwargs):
-            assert False
-
-        monkeypatch.setattr(subprocess, 'call', check_call_mock)
-
-        s = self.storage_class(str(tmpdir), '.txt', post_hook=None)
+    def test_post_hook_active(self, tmpdir):
+        s = self.storage_class(str(tmpdir), '.txt', post_hook='rm')
         s.upload(format_item('a/b/c'))
-
-    def test_post_hook_active(self, tmpdir, monkeypatch):
-
-        calls = []
-        exe = 'foo'
-
-        def check_call_mock(l, *args, **kwargs):
-            calls.append(True)
-            assert len(l) == 2
-            assert l[0] == exe
-
-        monkeypatch.setattr(subprocess, 'call', check_call_mock)
-
-        s = self.storage_class(str(tmpdir), '.txt', post_hook=exe)
-        s.upload(format_item('a/b/c'))
-        assert calls
+        assert not list(s.list())
 
     def test_ignore_git_dirs(self, tmpdir):
         tmpdir.mkdir('.git').mkdir('foo')

vdirsyncer/storage/filesystem.py

@@ -3,19 +3,18 @@
 import errno
 import logging
 import os
-import subprocess
 
 from atomicwrites import atomic_write
 
 from .base import Storage, normalize_meta_value
-from .. import exceptions
-from ..utils import checkdir, expand_path, generate_href, get_etag_from_file
-from ..vobject import Item
+from ._rust import RustStorageMixin
+from .. import native
+from ..utils import checkdir, expand_path
 
 logger = logging.getLogger(__name__)
 
 
-class FilesystemStorage(Storage):
+class FilesystemStorage(RustStorageMixin, Storage):
 
     '''
     Saves each item in its own file, given a directory.
@@ -52,6 +51,15 @@ class FilesystemStorage(Storage):
         self.fileext = fileext
         self.post_hook = post_hook
 
+        self._native_storage = native.ffi.gc(
+            native.lib.vdirsyncer_init_filesystem(
+                path.encode('utf-8'),
+                fileext.encode('utf-8'),
+                (post_hook or "").encode('utf-8')
+            ),
+            native.lib.vdirsyncer_storage_free
+        )
+
     @classmethod
     def discover(cls, path, **kwargs):
         if kwargs.pop('collection', None) is not None:
@@ -93,102 +101,6 @@ class FilesystemStorage(Storage):
         kwargs['collection'] = collection
         return kwargs
 
-    def _get_filepath(self, href):
-        return os.path.join(self.path, href)
-
-    def _get_href(self, ident):
-        return generate_href(ident) + self.fileext
-
-    def list(self):
-        for fname in os.listdir(self.path):
-            fpath = os.path.join(self.path, fname)
-            if os.path.isfile(fpath) and fname.endswith(self.fileext):
-                yield fname, get_etag_from_file(fpath)
-
-    def get(self, href):
-        fpath = self._get_filepath(href)
-        try:
-            with open(fpath, 'rb') as f:
-                return (Item(f.read().decode(self.encoding)),
-                        get_etag_from_file(fpath))
-        except IOError as e:
-            if e.errno == errno.ENOENT:
-                raise exceptions.NotFoundError(href)
-            else:
-                raise
-
-    def upload(self, item):
-        if not isinstance(item.raw, str):
-            raise TypeError('item.raw must be a unicode string.')
-
-        try:
-            href = self._get_href(item.ident)
-            fpath, etag = self._upload_impl(item, href)
-        except OSError as e:
-            if e.errno in (
-                errno.ENAMETOOLONG,  # Unix
-                errno.ENOENT  # Windows
-            ):
-                logger.debug('UID as filename rejected, trying with random '
-                             'one.')
-                # random href instead of UID-based
-                href = self._get_href(None)
-                fpath, etag = self._upload_impl(item, href)
-            else:
-                raise
-
-        if self.post_hook:
-            self._run_post_hook(fpath)
-        return href, etag
-
-    def _upload_impl(self, item, href):
-        fpath = self._get_filepath(href)
-        try:
-            with atomic_write(fpath, mode='wb', overwrite=False) as f:
-                f.write(item.raw.encode(self.encoding))
-                return fpath, get_etag_from_file(f)
-        except OSError as e:
-            if e.errno == errno.EEXIST:
-                raise exceptions.AlreadyExistingError(existing_href=href)
-            else:
-                raise
-
-    def update(self, href, item, etag):
-        fpath = self._get_filepath(href)
-        if not os.path.exists(fpath):
-            raise exceptions.NotFoundError(item.uid)
-        actual_etag = get_etag_from_file(fpath)
-        if etag != actual_etag:
-            raise exceptions.WrongEtagError(etag, actual_etag)
-
-        if not isinstance(item.raw, str):
-            raise TypeError('item.raw must be a unicode string.')
-
-        with atomic_write(fpath, mode='wb', overwrite=True) as f:
-            f.write(item.raw.encode(self.encoding))
-            etag = get_etag_from_file(f)
-
-        if self.post_hook:
-            self._run_post_hook(fpath)
-        return etag
-
-    def delete(self, href, etag):
-        fpath = self._get_filepath(href)
-        if not os.path.isfile(fpath):
-            raise exceptions.NotFoundError(href)
-        actual_etag = get_etag_from_file(fpath)
-        if etag != actual_etag:
-            raise exceptions.WrongEtagError(etag, actual_etag)
-        os.remove(fpath)
-
-    def _run_post_hook(self, fpath):
-        logger.info('Calling post_hook={} with argument={}'.format(
-            self.post_hook, fpath))
-        try:
-            subprocess.call([self.post_hook, fpath])
-        except OSError as e:
-            logger.warning('Error executing external hook: {}'.format(str(e)))
-
     def get_meta(self, key):
         fpath = os.path.join(self.path, key)
         try: