From 1b7cb4e65654573ce517f54cb83968cabae6ef11 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 4 Oct 2017 22:41:18 +0200 Subject: [PATCH] Use rust-vobject (#675) Use rust-vobject --- .gitignore | 1 + docs/installation.rst | 4 +- rust/.gitignore | 2 + rust/Cargo.toml | 15 +++ rust/build.rs | 12 +++ rust/src/lib.rs | 102 ++++++++++++++++++++ scripts/dpkg.Dockerfile | 7 +- scripts/travis-install.sh | 3 + setup.cfg | 2 +- setup.py | 30 +++++- tests/__init__.py | 11 ++- tests/storage/__init__.py | 12 +-- tests/storage/dav/test_caldav.py | 12 +-- tests/storage/test_filesystem.py | 10 +- tests/system/cli/test_repair.py | 2 +- tests/system/cli/test_sync.py | 36 ++++--- tests/unit/sync/test_sync.py | 156 +++++++++++++++++-------------- tests/unit/test_repair.py | 2 +- tests/unit/utils/test_vobject.py | 2 +- vdirsyncer/exceptions.py | 4 + vdirsyncer/native.py | 41 ++++++++ vdirsyncer/vobject.py | 40 ++++---- 22 files changed, 375 insertions(+), 131 deletions(-) create mode 100644 rust/.gitignore create mode 100644 rust/Cargo.toml create mode 100644 rust/build.rs create mode 100644 rust/src/lib.rs create mode 100644 vdirsyncer/native.py diff --git a/.gitignore b/.gitignore index 7538d58..c2c151c 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,5 @@ env dist docs/_build/ vdirsyncer/version.py +vdirsyncer/_native* .hypothesis diff --git a/docs/installation.rst b/docs/installation.rst index 0bc22d9..fb8758a 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -44,12 +44,14 @@ following things are installed: - Python 3.4+ and pip. - ``libxml`` and ``libxslt`` - ``zlib`` +- `Rust `, the programming language, together with + its package manager ``cargo``. - Linux or OS X. **Windows is not supported**, see :gh:`535`. On Linux systems, using the distro's package manager is the best way to do this, for example, using Ubuntu:: - sudo apt-get install libxml2 libxslt1.1 zlib1g python3 + sudo apt-get install libxml2 libxslt1.1 zlib1g python3 rustc cargo Then you have several options. The following text applies for most Python software by the way. diff --git a/rust/.gitignore b/rust/.gitignore new file mode 100644 index 0000000..1e7caa9 --- /dev/null +++ b/rust/.gitignore @@ -0,0 +1,2 @@ +Cargo.lock +target/ diff --git a/rust/Cargo.toml b/rust/Cargo.toml new file mode 100644 index 0000000..a03d9b2 --- /dev/null +++ b/rust/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "vdirsyncer_rustext" +version = "0.1.0" +authors = ["Markus Unterwaditzer "] +build = "build.rs" + +[lib] +name = "vdirsyncer_rustext" +crate-type = ["cdylib"] + +[dependencies] +vobject = "0.2.0" + +[build-dependencies] +cbindgen = "0.1" diff --git a/rust/build.rs b/rust/build.rs new file mode 100644 index 0000000..1c00f4c --- /dev/null +++ b/rust/build.rs @@ -0,0 +1,12 @@ +extern crate cbindgen; + +use std::env; + +fn main() { + let crate_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); + let mut config: cbindgen::Config = Default::default(); + config.language = cbindgen::Language::C; + cbindgen::generate_with_config(&crate_dir, config) + .unwrap() + .write_to_file("target/vdirsyncer_rustext.h"); +} diff --git a/rust/src/lib.rs b/rust/src/lib.rs new file mode 100644 index 0000000..ff67a60 --- /dev/null +++ b/rust/src/lib.rs @@ -0,0 +1,102 @@ +extern crate vobject; + +use std::ffi::{CStr, CString}; +use std::os::raw::c_char; +use std::mem; +use std::ptr; + +const EMPTY_STRING: *const c_char = b"\0" as *const u8 as *const c_char; + +#[repr(C)] +pub struct VdirsyncerError { + pub failed: bool, + pub msg: *mut c_char, +} + +// Workaround to be able to use opaque pointer +#[repr(C)] +pub struct VdirsyncerComponent(vobject::Component); + +#[no_mangle] +pub unsafe extern "C" fn vdirsyncer_get_uid(c: *mut VdirsyncerComponent) -> *const c_char { + match safe_get_uid(&(*c).0) { + Some(x) => CString::new(x).unwrap().into_raw(), + None => EMPTY_STRING + } +} + +#[inline] +fn safe_get_uid(c: &vobject::Component) -> Option { + let mut stack = vec![c]; + + while let Some(vobj) = stack.pop() { + if let Some(prop) = vobj.get_only("UID") { + return Some(prop.value_as_string()); + } + stack.extend(vobj.subcomponents.iter()); + }; + None +} + +#[no_mangle] +pub unsafe extern "C" fn vdirsyncer_free_str(s: *const c_char) { + CStr::from_ptr(s); +} + +#[no_mangle] +pub unsafe extern "C" fn vdirsyncer_parse_component(s: *const c_char, err: *mut VdirsyncerError) -> *mut VdirsyncerComponent { + let cstring = CStr::from_ptr(s); + match vobject::parse_component(cstring.to_str().unwrap()) { + Ok(x) => mem::transmute(Box::new(VdirsyncerComponent(x))), + Err(e) => { + (*err).failed = true; + (*err).msg = CString::new(e.into_string()).unwrap().into_raw(); + mem::zeroed() + } + } +} + +#[no_mangle] +pub unsafe extern "C" fn vdirsyncer_free_component(c: *mut VdirsyncerComponent) { + let _: Box = mem::transmute(c); +} + +#[no_mangle] +pub unsafe extern "C" fn vdirsyncer_clear_err(e: *mut VdirsyncerError) { + CString::from_raw((*e).msg); + (*e).msg = ptr::null_mut(); +} + +#[no_mangle] +pub unsafe extern "C" fn vdirsyncer_change_uid(c: *mut VdirsyncerComponent, uid: *const c_char) { + let uid_cstring = CStr::from_ptr(uid); + change_uid(&mut (*c).0, uid_cstring.to_str().unwrap()); +} + +fn change_uid(c: &mut vobject::Component, uid: &str) { + let mut stack = vec![c]; + while let Some(component) = stack.pop() { + match component.name.as_ref() { + "VEVENT" | "VTODO" | "VJOURNAL" | "VCARD" => { + if !uid.is_empty() { + component.set(vobject::Property::new("UID", uid)); + } else { + component.remove("UID"); + } + }, + _ => () + } + + stack.extend(component.subcomponents.iter_mut()); + } +} + +#[no_mangle] +pub unsafe extern "C" fn vdirsyncer_clone_component(c: *mut VdirsyncerComponent) -> *mut VdirsyncerComponent { + mem::transmute(Box::new((*c).0.clone())) +} + +#[no_mangle] +pub unsafe extern "C" fn vdirsyncer_write_component(c: *mut VdirsyncerComponent) -> *const c_char { + CString::new(vobject::write_component(&(*c).0)).unwrap().into_raw() +} diff --git a/scripts/dpkg.Dockerfile b/scripts/dpkg.Dockerfile index 78c8aab..f48783c 100644 --- a/scripts/dpkg.Dockerfile +++ b/scripts/dpkg.Dockerfile @@ -8,9 +8,12 @@ ARG distrover RUN apt-get update RUN apt-get install -y build-essential fakeroot debhelper git -RUN apt-get install -y python3-all python3-pip +RUN apt-get install -y python3-all python3-dev python3-pip RUN apt-get install -y ruby ruby-dev RUN apt-get install -y python-all python-pip +RUN curl https://sh.rustup.rs -sSf | sh -s -- -y +RUN apt-get install -y libssl-dev libffi-dev +ENV PATH="/root/.cargo/bin/:${PATH}" RUN gem install fpm @@ -24,7 +27,7 @@ RUN mkdir /vdirsyncer/pkgs/ RUN basename *.tar.gz .tar.gz | cut -d'-' -f2 | sed -e 's/\.dev/~/g' | tee version RUN (echo -n *.tar.gz; echo '[google]') | tee requirements.txt -RUN . /vdirsyncer/env/bin/activate; fpm -s virtualenv -t deb \ +RUN . /vdirsyncer/env/bin/activate; fpm --verbose -s virtualenv -t deb \ -n "vdirsyncer-latest" \ -v "$(cat version)" \ --prefix /opt/venvs/vdirsyncer-latest \ diff --git a/scripts/travis-install.sh b/scripts/travis-install.sh index 9b78409..a9be183 100644 --- a/scripts/travis-install.sh +++ b/scripts/travis-install.sh @@ -8,3 +8,6 @@ if [ "$TRAVIS_OS_NAME" = "osx" ]; then virtualenv -p python3 $HOME/osx-py3 . $HOME/osx-py3/bin/activate fi + +curl https://sh.rustup.rs -sSf | sh -s -- -y +export PATH="$HOME/.cargo/bin/:$PATH" diff --git a/setup.cfg b/setup.cfg index 1909b8d..fbbf4db 100644 --- a/setup.cfg +++ b/setup.cfg @@ -9,5 +9,5 @@ addopts = --tb=short # E731: Use a def instead of lambda expr ignore = E731 select = C,E,F,W,B,B9 -exclude = tests/storage/servers/owncloud/, tests/storage/servers/nextcloud/, tests/storage/servers/baikal/, build/ +exclude = .eggs/, tests/storage/servers/owncloud/, tests/storage/servers/nextcloud/, tests/storage/servers/baikal/, build/, vdirsyncer/_native* application-package-names = tests,vdirsyncer diff --git a/setup.py b/setup.py index b033ce9..f8d641c 100644 --- a/setup.py +++ b/setup.py @@ -9,6 +9,7 @@ how to package vdirsyncer. from setuptools import Command, find_packages, setup +milksnake = 'milksnake' requirements = [ # https://github.com/mitsuhiko/click/issues/200 @@ -32,10 +33,29 @@ requirements = [ 'requests_toolbelt >=0.4.0', # https://github.com/untitaker/python-atomicwrites/commit/4d12f23227b6a944ab1d99c507a69fdbc7c9ed6d # noqa - 'atomicwrites>=0.1.7' + 'atomicwrites>=0.1.7', + milksnake ] +def build_native(spec): + build = spec.add_external_build( + cmd=['cargo', 'build', '--release'], + path='./rust' + ) + + spec.add_cffi_module( + module_path='vdirsyncer._native', + dylib=lambda: build.find_dylib( + 'vdirsyncer_rustext', in_path='target/release'), + header_filename=lambda: build.find_header( + 'vdirsyncer_rustext.h', in_path='target'), + # Rust bug: If thread-local storage is used, this flag is necessary + # (mitsuhiko) + rtld_flags=['NODELETE'] + ) + + class PrintRequirements(Command): description = 'Prints minimal requirements' user_options = [] @@ -75,7 +95,10 @@ setup( }, # Build dependencies - setup_requires=['setuptools_scm != 1.12.0'], + setup_requires=[ + 'setuptools_scm != 1.12.0', + milksnake, + ], # Other packages=find_packages(exclude=['tests.*', 'tests']), @@ -101,4 +124,7 @@ setup( 'Topic :: Internet', 'Topic :: Utilities', ], + milksnake_tasks=[build_native], + zip_safe=False, + platforms='any' ) diff --git a/tests/__init__.py b/tests/__init__.py index 9103d74..0f94351 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -3,9 +3,11 @@ Test suite for vdirsyncer. ''' +import random + import hypothesis.strategies as st -from vdirsyncer.vobject import normalize_item +from vdirsyncer.vobject import Item, normalize_item import urllib3 import urllib3.exceptions @@ -109,3 +111,10 @@ uid_strategy = st.text( )), min_size=1 ).filter(lambda x: x.strip() == x) + + +def format_item(uid=None, item_template=VCARD_TEMPLATE): + # assert that special chars are handled correctly. + r = random.random() + uid = uid or r + return Item(item_template.format(r=r, uid=uid)) diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index a66c008..9fef293 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -1,6 +1,5 @@ # -*- coding: utf-8 -*- -import random import uuid import textwrap @@ -16,7 +15,8 @@ from vdirsyncer.storage.base import normalize_meta_value from vdirsyncer.vobject import Item from .. import EVENT_TEMPLATE, TASK_TEMPLATE, VCARD_TEMPLATE, \ - assert_item_equals, normalize_item, printable_characters_strategy + assert_item_equals, format_item, normalize_item, \ + printable_characters_strategy def get_server_mixin(server_name): @@ -25,12 +25,6 @@ def get_server_mixin(server_name): return x.ServerMixin -def format_item(item_template, uid=None): - # assert that special chars are handled correctly. - r = random.random() - return Item(item_template.format(r=r, uid=uid or r)) - - class StorageTests(object): storage_class = None supports_collections = True @@ -62,7 +56,7 @@ class StorageTests(object): 'VCARD': VCARD_TEMPLATE, }[item_type] - return lambda **kw: format_item(template, **kw) + return lambda **kw: format_item(item_template=template, **kw) @pytest.fixture def requires_collections(self): diff --git a/tests/storage/dav/test_caldav.py b/tests/storage/dav/test_caldav.py index 3063bbd..ed5657d 100644 --- a/tests/storage/dav/test_caldav.py +++ b/tests/storage/dav/test_caldav.py @@ -28,7 +28,7 @@ class TestCalDAVStorage(DAVStorageTests): s = self.storage_class(item_types=(item_type,), **get_storage_args()) try: - s.upload(format_item(VCARD_TEMPLATE)) + s.upload(format_item(item_template=VCARD_TEMPLATE)) except (exceptions.Error, requests.exceptions.HTTPError): pass assert not list(s.list()) @@ -64,7 +64,7 @@ class TestCalDAVStorage(DAVStorageTests): s = self.storage_class(start_date=start_date, end_date=end_date, **get_storage_args()) - too_old_item = format_item(dedent(u''' + too_old_item = format_item(item_template=dedent(u''' BEGIN:VCALENDAR VERSION:2.0 PRODID:-//hacksw/handcal//NONSGML v1.0//EN @@ -78,7 +78,7 @@ class TestCalDAVStorage(DAVStorageTests): END:VCALENDAR ''').strip()) - too_new_item = format_item(dedent(u''' + too_new_item = format_item(item_template=dedent(u''' BEGIN:VCALENDAR VERSION:2.0 PRODID:-//hacksw/handcal//NONSGML v1.0//EN @@ -92,7 +92,7 @@ class TestCalDAVStorage(DAVStorageTests): END:VCALENDAR ''').strip()) - good_item = format_item(dedent(u''' + good_item = format_item(item_template=dedent(u''' BEGIN:VCALENDAR VERSION:2.0 PRODID:-//hacksw/handcal//NONSGML v1.0//EN @@ -136,8 +136,8 @@ class TestCalDAVStorage(DAVStorageTests): @pytest.mark.skipif(dav_server == 'icloud', reason='iCloud only accepts VEVENT') def test_item_types_general(self, s): - event = s.upload(format_item(EVENT_TEMPLATE))[0] - task = s.upload(format_item(TASK_TEMPLATE))[0] + event = s.upload(format_item(item_template=EVENT_TEMPLATE))[0] + task = s.upload(format_item(item_template=TASK_TEMPLATE))[0] s.item_types = ('VTODO', 'VEVENT') def l(): diff --git a/tests/storage/test_filesystem.py b/tests/storage/test_filesystem.py index 3dcd5ff..5501720 100644 --- a/tests/storage/test_filesystem.py +++ b/tests/storage/test_filesystem.py @@ -5,9 +5,9 @@ import subprocess import pytest from vdirsyncer.storage.filesystem import FilesystemStorage -from vdirsyncer.vobject import Item from . import StorageTests +from tests import format_item class TestFilesystemStorage(StorageTests): @@ -42,13 +42,13 @@ class TestFilesystemStorage(StorageTests): def test_ident_with_slash(self, tmpdir): s = self.storage_class(str(tmpdir), '.txt') - s.upload(Item(u'UID:a/b/c')) + s.upload(format_item('a/b/c')) item_file, = tmpdir.listdir() assert '/' not in item_file.basename and item_file.isfile() def test_too_long_uid(self, tmpdir): s = self.storage_class(str(tmpdir), '.txt') - item = Item(u'UID:' + u'hue' * 600) + item = format_item('hue' * 600) href, etag = s.upload(item) assert item.uid not in href @@ -60,7 +60,7 @@ class TestFilesystemStorage(StorageTests): monkeypatch.setattr(subprocess, 'call', check_call_mock) s = self.storage_class(str(tmpdir), '.txt', post_hook=None) - s.upload(Item(u'UID:a/b/c')) + s.upload(format_item('a/b/c')) def test_post_hook_active(self, tmpdir, monkeypatch): @@ -75,7 +75,7 @@ class TestFilesystemStorage(StorageTests): monkeypatch.setattr(subprocess, 'call', check_call_mock) s = self.storage_class(str(tmpdir), '.txt', post_hook=exe) - s.upload(Item(u'UID:a/b/c')) + s.upload(format_item('a/b/c')) assert calls def test_ignore_git_dirs(self, tmpdir): diff --git a/tests/system/cli/test_repair.py b/tests/system/cli/test_repair.py index 7573801..b7c9a1f 100644 --- a/tests/system/cli/test_repair.py +++ b/tests/system/cli/test_repair.py @@ -62,7 +62,7 @@ def test_repair_uids(storage, runner, repair_uids): assert 'UID or href is unsafe, assigning random UID' in result.output assert not f.exists() new_f, = storage.listdir() - s = new_f.read() + s = new_f.read().strip() assert s.startswith('BEGIN:VCARD') assert s.endswith('END:VCARD') diff --git a/tests/system/cli/test_sync.py b/tests/system/cli/test_sync.py index e8ddca9..46afcb5 100644 --- a/tests/system/cli/test_sync.py +++ b/tests/system/cli/test_sync.py @@ -9,6 +9,8 @@ from hypothesis import example, given import pytest +from tests import format_item + def test_simple_run(tmpdir, runner): runner.write_with_general(dedent(''' @@ -37,10 +39,11 @@ def test_simple_run(tmpdir, runner): result = runner.invoke(['sync']) assert not result.exception - tmpdir.join('path_a/haha.txt').write('UID:haha') + item = format_item('haha') + tmpdir.join('path_a/haha.txt').write(item.raw) result = runner.invoke(['sync']) assert 'Copying (uploading) item haha to my_b' in result.output - assert tmpdir.join('path_b/haha.txt').read() == 'UID:haha' + assert tmpdir.join('path_b/haha.txt').read() == item.raw def test_sync_inexistant_pair(tmpdir, runner): @@ -109,7 +112,8 @@ def test_empty_storage(tmpdir, runner): result = runner.invoke(['sync']) assert not result.exception - tmpdir.join('path_a/haha.txt').write('UID:haha') + item = format_item('haha') + tmpdir.join('path_a/haha.txt').write(item.raw) result = runner.invoke(['sync']) assert not result.exception tmpdir.join('path_b/haha.txt').remove() @@ -152,7 +156,7 @@ def test_collections_cache_invalidation(tmpdir, runner): collections = ["a", "b", "c"] ''').format(str(tmpdir))) - foo.join('a/itemone.txt').write('UID:itemone') + foo.join('a/itemone.txt').write(format_item('itemone').raw) result = runner.invoke(['discover']) assert not result.exception @@ -347,9 +351,10 @@ def test_ident_conflict(tmpdir, runner): foo = tmpdir.mkdir('foo') tmpdir.mkdir('bar') - foo.join('one.txt').write('UID:1') - foo.join('two.txt').write('UID:1') - foo.join('three.txt').write('UID:1') + item = format_item('1') + foo.join('one.txt').write(item.raw) + foo.join('two.txt').write(item.raw) + foo.join('three.txt').write(item.raw) result = runner.invoke(['discover']) assert not result.exception @@ -403,8 +408,12 @@ def test_no_configured_pairs(tmpdir, runner, cmd): assert result.exception.code == 5 +item_a = format_item('lol') +item_b = format_item('lol') + + @pytest.mark.parametrize('resolution,expect_foo,expect_bar', [ - (['command', 'cp'], 'UID:lol\nfööcontent', 'UID:lol\nfööcontent') + (['command', 'cp'], item_a.raw, item_a.raw) ]) def test_conflict_resolution(tmpdir, runner, resolution, expect_foo, expect_bar): @@ -429,9 +438,9 @@ def test_conflict_resolution(tmpdir, runner, resolution, expect_foo, foo = tmpdir.join('foo') bar = tmpdir.join('bar') fooitem = foo.join('lol.txt').ensure() - fooitem.write('UID:lol\nfööcontent') + fooitem.write(item_a.raw) baritem = bar.join('lol.txt').ensure() - baritem.write('UID:lol\nbööcontent') + baritem.write(item_b.raw) r = runner.invoke(['discover']) assert not r.exception @@ -471,11 +480,12 @@ def test_partial_sync(tmpdir, runner, partial_sync): foo = tmpdir.mkdir('foo') bar = tmpdir.mkdir('bar') - foo.join('other.txt').write('UID:other') - bar.join('other.txt').write('UID:other') + item = format_item('other') + foo.join('other.txt').write(item.raw) + bar.join('other.txt').write(item.raw) baritem = bar.join('lol.txt') - baritem.write('UID:lol') + baritem.write(format_item('lol').raw) r = runner.invoke(['discover']) assert not r.exception diff --git a/tests/unit/sync/test_sync.py b/tests/unit/sync/test_sync.py index ca96f71..fa3d013 100644 --- a/tests/unit/sync/test_sync.py +++ b/tests/unit/sync/test_sync.py @@ -8,7 +8,7 @@ from hypothesis.stateful import Bundle, RuleBasedStateMachine, rule import pytest -from tests import blow_up, uid_strategy +from tests import blow_up, format_item, uid_strategy from vdirsyncer.storage.memory import MemoryStorage, _random_string from vdirsyncer.sync import sync as _sync @@ -49,7 +49,7 @@ def test_missing_status(): a = MemoryStorage() b = MemoryStorage() status = {} - item = Item(u'asdf') + item = format_item('asdf') a.upload(item) b.upload(item) sync(a, b, status) @@ -62,8 +62,8 @@ def test_missing_status_and_different_items(): b = MemoryStorage() status = {} - item1 = Item(u'UID:1\nhaha') - item2 = Item(u'UID:1\nhoho') + item1 = format_item('1') + item2 = format_item('1') a.upload(item1) b.upload(item2) with pytest.raises(SyncConflict): @@ -79,8 +79,8 @@ def test_read_only_and_prefetch(): b.read_only = True status = {} - item1 = Item(u'UID:1\nhaha') - item2 = Item(u'UID:2\nhoho') + item1 = format_item('1') + item2 = format_item('2') a.upload(item1) a.upload(item2) @@ -95,7 +95,8 @@ def test_partial_sync_error(): b = MemoryStorage() status = {} - a.upload(Item('UID:0')) + item = format_item('0') + a.upload(item) b.read_only = True with pytest.raises(PartialSync): @@ -107,13 +108,13 @@ def test_partial_sync_ignore(): b = MemoryStorage() status = {} - item0 = Item('UID:0\nhehe') + item0 = format_item('0') a.upload(item0) b.upload(item0) b.read_only = True - item1 = Item('UID:1\nhaha') + item1 = format_item('1') a.upload(item1) sync(a, b, status, partial_sync='ignore') @@ -128,23 +129,25 @@ def test_partial_sync_ignore2(): b = MemoryStorage() status = {} - href, etag = a.upload(Item('UID:0')) + item = format_item('0') + href, etag = a.upload(item) a.read_only = True sync(a, b, status, partial_sync='ignore', force_delete=True) - assert items(b) == items(a) == {'UID:0'} + assert items(b) == items(a) == {item.raw} b.items.clear() sync(a, b, status, partial_sync='ignore', force_delete=True) sync(a, b, status, partial_sync='ignore', force_delete=True) - assert items(a) == {'UID:0'} + assert items(a) == {item.raw} assert not b.items a.read_only = False - a.update(href, Item('UID:0\nupdated'), etag) + new_item = format_item('0') + a.update(href, new_item, etag) a.read_only = True sync(a, b, status, partial_sync='ignore', force_delete=True) - assert items(b) == items(a) == {'UID:0\nupdated'} + assert items(b) == items(a) == {new_item.raw} def test_upload_and_update(): @@ -152,22 +155,22 @@ def test_upload_and_update(): b = MemoryStorage(fileext='.b') status = {} - item = Item(u'UID:1') # new item 1 in a + item = format_item('1') # new item 1 in a a.upload(item) sync(a, b, status) assert items(b) == items(a) == {item.raw} - item = Item(u'UID:1\nASDF:YES') # update of item 1 in b + item = format_item('1') # update of item 1 in b b.update('1.b', item, b.get('1.b')[1]) sync(a, b, status) assert items(b) == items(a) == {item.raw} - item2 = Item(u'UID:2') # new item 2 in b + item2 = format_item('2') # new item 2 in b b.upload(item2) sync(a, b, status) assert items(b) == items(a) == {item.raw, item2.raw} - item2 = Item(u'UID:2\nASDF:YES') # update of item 2 in a + item2 = format_item('2') # update of item 2 in a a.update('2.a', item2, a.get('2.a')[1]) sync(a, b, status) assert items(b) == items(a) == {item.raw, item2.raw} @@ -178,9 +181,9 @@ def test_deletion(): b = MemoryStorage(fileext='.b') status = {} - item = Item(u'UID:1') + item = format_item('1') a.upload(item) - item2 = Item(u'UID:2') + item2 = format_item('2') a.upload(item2) sync(a, b, status) b.delete('1.b', b.get('1.b')[1]) @@ -200,14 +203,14 @@ def test_insert_hash(): b = MemoryStorage() status = {} - item = Item('UID:1') + item = format_item('1') href, etag = a.upload(item) sync(a, b, status) for d in status['1']: del d['hash'] - a.update(href, Item('UID:1\nHAHA:YES'), etag) + a.update(href, format_item('1'), etag) # new item content sync(a, b, status) assert 'hash' in status['1'][0] and 'hash' in status['1'][1] @@ -215,7 +218,7 @@ def test_insert_hash(): def test_already_synced(): a = MemoryStorage(fileext='.a') b = MemoryStorage(fileext='.b') - item = Item(u'UID:1') + item = format_item('1') a.upload(item) b.upload(item) status = { @@ -243,14 +246,14 @@ def test_already_synced(): def test_conflict_resolution_both_etags_new(winning_storage): a = MemoryStorage() b = MemoryStorage() - item = Item(u'UID:1') + item = format_item('1') href_a, etag_a = a.upload(item) href_b, etag_b = b.upload(item) status = {} sync(a, b, status) assert status - item_a = Item(u'UID:1\nitem a') - item_b = Item(u'UID:1\nitem b') + item_a = format_item('1') + item_b = format_item('1') a.update(href_a, item_a, etag_a) b.update(href_b, item_b, etag_b) with pytest.raises(SyncConflict): @@ -264,13 +267,14 @@ def test_conflict_resolution_both_etags_new(winning_storage): def test_updated_and_deleted(): a = MemoryStorage() b = MemoryStorage() - href_a, etag_a = a.upload(Item(u'UID:1')) + item = format_item('1') + href_a, etag_a = a.upload(item) status = {} sync(a, b, status, force_delete=True) (href_b, etag_b), = b.list() b.delete(href_b, etag_b) - updated = Item(u'UID:1\nupdated') + updated = format_item('1') a.update(href_a, updated, etag_a) sync(a, b, status, force_delete=True) @@ -280,8 +284,8 @@ def test_updated_and_deleted(): def test_conflict_resolution_invalid_mode(): a = MemoryStorage() b = MemoryStorage() - item_a = Item(u'UID:1\nitem a') - item_b = Item(u'UID:1\nitem b') + item_a = format_item('1') + item_b = format_item('1') a.upload(item_a) b.upload(item_b) with pytest.raises(ValueError): @@ -291,7 +295,7 @@ def test_conflict_resolution_invalid_mode(): def test_conflict_resolution_new_etags_without_changes(): a = MemoryStorage() b = MemoryStorage() - item = Item(u'UID:1') + item = format_item('1') href_a, etag_a = a.upload(item) href_b, etag_b = b.upload(item) status = {'1': (href_a, 'BOGUS_a', href_b, 'BOGUS_b')} @@ -326,7 +330,7 @@ def test_uses_get_multi(monkeypatch): a = MemoryStorage() b = MemoryStorage() - item = Item(u'UID:1') + item = format_item('1') expected_href, etag = a.upload(item) sync(a, b, {}) @@ -336,8 +340,8 @@ def test_uses_get_multi(monkeypatch): def test_empty_storage_dataloss(): a = MemoryStorage() b = MemoryStorage() - a.upload(Item(u'UID:1')) - a.upload(Item(u'UID:2')) + for i in '12': + a.upload(format_item(i)) status = {} sync(a, b, status) with pytest.raises(StorageEmpty): @@ -350,22 +354,24 @@ def test_empty_storage_dataloss(): def test_no_uids(): a = MemoryStorage() b = MemoryStorage() - a.upload(Item(u'ASDF')) - b.upload(Item(u'FOOBAR')) + item_a = format_item('') + item_b = format_item('') + a.upload(item_a) + b.upload(item_b) status = {} sync(a, b, status) - assert items(a) == items(b) == {u'ASDF', u'FOOBAR'} + assert items(a) == items(b) == {item_a.raw, item_b.raw} def test_changed_uids(): a = MemoryStorage() b = MemoryStorage() - href_a, etag_a = a.upload(Item(u'UID:A-ONE')) - href_b, etag_b = b.upload(Item(u'UID:B-ONE')) + href_a, etag_a = a.upload(format_item('a1')) + href_b, etag_b = b.upload(format_item('b1')) status = {} sync(a, b, status) - a.update(href_a, Item(u'UID:A-TWO'), etag_a) + a.update(href_a, format_item('a2'), etag_a) sync(a, b, status) @@ -383,34 +389,37 @@ def test_partial_sync_revert(): a = MemoryStorage(instance_name='a') b = MemoryStorage(instance_name='b') status = {} - a.upload(Item(u'UID:1')) - b.upload(Item(u'UID:2')) + item1 = format_item('1') + item2 = format_item('2') + a.upload(item1) + b.upload(item2) b.read_only = True sync(a, b, status, partial_sync='revert') assert len(status) == 2 - assert items(a) == {'UID:1', 'UID:2'} - assert items(b) == {'UID:2'} + assert items(a) == {item1.raw, item2.raw} + assert items(b) == {item2.raw} sync(a, b, status, partial_sync='revert') assert len(status) == 1 - assert items(a) == {'UID:2'} - assert items(b) == {'UID:2'} + assert items(a) == {item2.raw} + assert items(b) == {item2.raw} # Check that updates get reverted - a.items[next(iter(a.items))] = ('foo', Item('UID:2\nupdated')) - assert items(a) == {'UID:2\nupdated'} + item2_up = format_item('2') + a.items[next(iter(a.items))] = ('foo', item2_up) + assert items(a) == {item2_up.raw} sync(a, b, status, partial_sync='revert') assert len(status) == 1 - assert items(a) == {'UID:2\nupdated'} + assert items(a) == {item2_up.raw} sync(a, b, status, partial_sync='revert') - assert items(a) == {'UID:2'} + assert items(a) == {item2.raw} # Check that deletions get reverted a.items.clear() sync(a, b, status, partial_sync='revert', force_delete=True) sync(a, b, status, partial_sync='revert', force_delete=True) - assert items(a) == {'UID:2'} + assert items(a) == {item2.raw} @pytest.mark.parametrize('sync_inbetween', (True, False)) @@ -418,13 +427,16 @@ def test_ident_conflict(sync_inbetween): a = MemoryStorage() b = MemoryStorage() status = {} - href_a, etag_a = a.upload(Item(u'UID:aaa')) - href_b, etag_b = a.upload(Item(u'UID:bbb')) + item_a = format_item('aaa') + item_b = format_item('bbb') + href_a, etag_a = a.upload(item_a) + href_b, etag_b = a.upload(item_b) if sync_inbetween: sync(a, b, status) - a.update(href_a, Item(u'UID:xxx'), etag_a) - a.update(href_b, Item(u'UID:xxx'), etag_b) + item_x = format_item('xxx') + a.update(href_a, item_x, etag_a) + a.update(href_b, item_x, etag_b) with pytest.raises(IdentConflict): sync(a, b, status) @@ -441,7 +453,8 @@ def test_moved_href(): a = MemoryStorage() b = MemoryStorage() status = {} - href, etag = a.upload(Item(u'UID:haha')) + item = format_item('haha') + href, etag = a.upload(item) sync(a, b, status) b.items['lol'] = b.items.pop('haha') @@ -454,7 +467,7 @@ def test_moved_href(): sync(a, b, status) assert len(status) == 1 - assert items(a) == items(b) == {'UID:haha'} + assert items(a) == items(b) == {item.raw} assert status['haha'][1]['href'] == 'lol' old_status = deepcopy(status) @@ -463,7 +476,7 @@ def test_moved_href(): sync(a, b, status) assert old_status == status - assert items(a) == items(b) == {'UID:haha'} + assert items(a) == items(b) == {item.raw} def test_bogus_etag_change(): @@ -476,26 +489,31 @@ def test_bogus_etag_change(): a = MemoryStorage() b = MemoryStorage() status = {} - href_a, etag_a = a.upload(Item(u'UID:ASDASD')) - sync(a, b, status) - assert len(status) == len(list(a.list())) == len(list(b.list())) == 1 + item = format_item('ASDASD') + href_a, etag_a = a.upload(item) + sync(a, b, status) + assert len(status) == 1 + assert items(a) == items(b) == {item.raw} + + new_item = format_item('ASDASD') (href_b, etag_b), = b.list() - a.update(href_a, Item(u'UID:ASDASD'), etag_a) - b.update(href_b, Item(u'UID:ASDASD\nACTUALCHANGE:YES'), etag_b) + a.update(href_a, item, etag_a) + b.update(href_b, new_item, etag_b) b.delete = b.update = b.upload = blow_up sync(a, b, status) assert len(status) == 1 - assert items(a) == items(b) == {u'UID:ASDASD\nACTUALCHANGE:YES'} + assert items(a) == items(b) == {new_item.raw} def test_unicode_hrefs(): a = MemoryStorage() b = MemoryStorage() status = {} - href, etag = a.upload(Item(u'UID:äää')) + item = format_item('äää') + href, etag = a.upload(item) sync(a, b, status) @@ -565,7 +583,7 @@ class SyncMachine(RuleBasedStateMachine): uid=uid_strategy, etag=st.text()) def upload(self, storage, uid, etag): - item = Item(u'UID:{}'.format(uid)) + item = Item('BEGIN:VCARD\r\nUID:{}\r\nEND:VCARD'.format(uid)) storage.items[uid] = (etag, item) @rule(storage=Storage, href=st.text()) @@ -643,8 +661,8 @@ def test_rollback(error_callback): b = MemoryStorage() status = {} - a.items['0'] = ('', Item('UID:0')) - b.items['1'] = ('', Item('UID:1')) + a.items['0'] = ('', format_item('0')) + b.items['1'] = ('', format_item('1')) b.upload = b.update = b.delete = action_failure @@ -668,7 +686,7 @@ def test_duplicate_hrefs(): a = MemoryStorage() b = MemoryStorage() a.list = lambda: [('a', 'a')] * 3 - a.items['a'] = ('a', Item('UID:a')) + a.items['a'] = ('a', format_item('a')) status = {} sync(a, b, status) diff --git a/tests/unit/test_repair.py b/tests/unit/test_repair.py index 0ba726e..457bf69 100644 --- a/tests/unit/test_repair.py +++ b/tests/unit/test_repair.py @@ -38,7 +38,7 @@ def test_repair_uids(uid): @settings(perform_health_check=False) # Using the random module for UIDs def test_repair_unsafe_uids(uid): s = MemoryStorage() - item = Item(u'BEGIN:VCARD\nUID:{}\nEND:VCARD'.format(uid)) + item = Item(u'BEGIN:VCARD\nUID:123\nEND:VCARD').with_uid(uid) href, etag = s.upload(item) assert s.get(href)[0].uid == uid assert not href_safe(uid) diff --git a/tests/unit/utils/test_vobject.py b/tests/unit/utils/test_vobject.py index cfb52d1..100756a 100644 --- a/tests/unit/utils/test_vobject.py +++ b/tests/unit/utils/test_vobject.py @@ -223,7 +223,7 @@ def test_replace_uid(template, uid): item = vobject.Item(template.format(r=123, uid=123)).with_uid(uid) assert item.uid == uid if uid: - assert item.raw.count('\nUID:{}'.format(uid)) == 1 + assert item.raw.count('\nUID:') == 1 else: assert '\nUID:' not in item.raw diff --git a/vdirsyncer/exceptions.py b/vdirsyncer/exceptions.py index 722e7de..3346cc3 100644 --- a/vdirsyncer/exceptions.py +++ b/vdirsyncer/exceptions.py @@ -79,3 +79,7 @@ class UnsupportedMetadataError(Error, NotImplementedError): class CollectionRequired(Error): '''`collection = null` is not allowed.''' + + +class VobjectParseError(Error, ValueError): + '''The parsed vobject is invalid.''' diff --git a/vdirsyncer/native.py b/vdirsyncer/native.py new file mode 100644 index 0000000..6eb54ea --- /dev/null +++ b/vdirsyncer/native.py @@ -0,0 +1,41 @@ +from ._native import ffi, lib +from .exceptions import VobjectParseError + + +def parse_component(raw): + e = ffi.new('VdirsyncerError *') + try: + c = lib.vdirsyncer_parse_component(raw, e) + if e.failed: + raise VobjectParseError(ffi.string(e.msg).decode('utf-8')) + return _component_rv(c) + finally: + if e.failed: + lib.vdirsyncer_clear_err(e) + + +def write_component(component): + return _string_rv(lib.vdirsyncer_write_component(component)) + + +def get_uid(component): + return _string_rv(lib.vdirsyncer_get_uid(component)) + + +def _string_rv(c_str): + try: + return ffi.string(c_str).decode('utf-8') + finally: + lib.vdirsyncer_free_str(c_str) + + +def change_uid(component, uid): + lib.vdirsyncer_change_uid(component, uid.encode('utf-8')) + + +def _component_rv(c): + return ffi.gc(c, lib.vdirsyncer_free_component) + + +def clone_component(c): + return _component_rv(lib.vdirsyncer_clone_component(c)) diff --git a/vdirsyncer/vobject.py b/vdirsyncer/vobject.py index d97a057..397bea8 100644 --- a/vdirsyncer/vobject.py +++ b/vdirsyncer/vobject.py @@ -4,6 +4,7 @@ import hashlib from itertools import chain, tee from .utils import cached_property, uniq +from . import exceptions, native IGNORE_PROPS = ( @@ -39,25 +40,25 @@ class Item(object): '''Immutable wrapper class for VCALENDAR (VEVENT, VTODO) and VCARD''' - def __init__(self, raw): + def __init__(self, raw, component=None): assert isinstance(raw, str), type(raw) self._raw = raw + try: + self._component = component or \ + native.parse_component(self.raw.encode('utf-8')) + except exceptions.VobjectParseError: + self._component = None + def with_uid(self, new_uid): - parsed = _Component.parse(self.raw) - stack = [parsed] - while stack: - component = stack.pop() - stack.extend(component.subcomponents) + if not self._component: + raise ValueError('Item malformed.') - if component.name in ('VEVENT', 'VTODO', 'VJOURNAL', 'VCARD'): - del component['UID'] - if new_uid: - component['UID'] = new_uid + new_c = native.clone_component(self._component) + native.change_uid(new_c, new_uid or '') + return Item(native.write_component(new_c), component=new_c) - return Item('\r\n'.join(parsed.dump_lines())) - - @cached_property + @property def raw(self): '''Raw content of the item, as unicode string. @@ -69,13 +70,9 @@ class Item(object): def uid(self): '''Global identifier of the item, across storages, doesn't change after a modification of the item.''' - # Don't actually parse component, but treat all lines as single - # component, avoiding traversal through all subcomponents. - x = _Component('TEMP', self.raw.splitlines(), []) - try: - return x['UID'].strip() or None - except KeyError: + if not self._component: return None + return native.get_uid(self._component) or None @cached_property def hash(self): @@ -99,11 +96,16 @@ class Item(object): @property def parsed(self): '''Don't cache because the rv is mutable.''' + # FIXME: remove try: return _Component.parse(self.raw) except Exception: return None + @property + def is_valid(self): + return bool(self._component) + def normalize_item(item, ignore_props=IGNORE_PROPS): '''Create syntactically invalid mess that is equal for similar items.'''