Skip to content

Commit fd52852

Browse files
committed
Move implicit config from storage to pair and add conflict resolution
Use conflict_resolution parameter to determine behavior when both 'create' and 'delete' are specified in implicit. When collection exists in A but not B: - 'a wins': create in B, delete from A - 'b wins': create in A, delete from B - No resolution: raise clear error This resolves the ambiguity issue raised in PR pimutils#869 by using the existing conflict_resolution mechanism to make the behavior predictable and controllable.
1 parent ac58378 commit fd52852

File tree

10 files changed

+478
-94
lines changed

10 files changed

+478
-94
lines changed

CHANGELOG.rst

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ Version 0.19.0
5656
- Add a new ``showconfig`` status. This prints *some* configuration values as
5757
JSON. This is intended to be used by external tools and helpers that interact
5858
with ``vdirsyncer``, and considered experimental.
59+
- **BREAKING**: Improved ``implicit`` parameter behavior when both "create" and
60+
"delete" are specified. Previously, the behavior in ambiguous cases (when a
61+
collection exists on one side but not the other) was undefined. Now, the
62+
``conflict_resolution`` parameter is used to determine the action: "a wins"
63+
creates missing collections in B and deletes extra collections from A, while
64+
"b wins" does the opposite. If both "create" and "delete" are specified
65+
without ``conflict_resolution``, vdirsyncer will raise an error instead of
66+
making unpredictable choices.
5967
- Update TLS-related tests that were failing due to weak MDs. :gh:`903`
6068
- ``pytest-httpserver`` and ``trustme`` are now required for tests.
6169
- ``pytest-localserver`` is no longer required for tests.
@@ -73,12 +81,14 @@ Version 0.19.0
7381
- The ``plist`` for macOS has been dropped. It was broken and homebrew
7482
generates their own based on package metadata. macOS users are encouraged to
7583
use that as a reference.
84+
- Add ``implicit`` option to the :ref:`pair section <pair_config>`. It implicitly
85+
creates and deletes collections in the destinations, when new collections are
86+
created/deleted in the source. The deletion is implemented only for the
87+
"filesystem" storage. This respects your `conflict_resolution` configuration
88+
and shows an error when not configured or set to run a command.
89+
See :ref:`implicit docs <implicit_def>` for details.
7690

7791
.. _etesync-dav: https://github.com/etesync/etesync-dav
78-
- Add ``implicit`` option to storage section. It creates/deletes implicitly
79-
collections in the destinations, when new collections are created/deleted
80-
in the source. The deletion is implemented only for the "filesystem" storage.
81-
See :ref:`storage_config`.
8292

8393
Changes to SSL configuration
8494
----------------------------

CLAUDE.md

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
## Project Overview
6+
7+
vdirsyncer is a command-line tool for synchronizing calendars and addressbooks between various servers and the local filesystem. It's essentially "OfflineIMAP for calendars and contacts" - syncing CalDAV and CardDAV servers with local vdir directories.
8+
9+
## Development Commands
10+
11+
### Setup and Installation
12+
```bash
13+
make install-dev # Install in development mode with test dependencies
14+
```
15+
16+
### Testing
17+
```bash
18+
make test # Run all tests
19+
make style # Check code style with flake8
20+
make test-storage # Run only storage tests
21+
make test-unit # Run only unit tests
22+
make test-system # Run only system tests
23+
24+
# Test against specific DAV servers:
25+
make DAV_SERVER=radicale test
26+
make DAV_SERVER=xandikos test
27+
make DAV_SERVER=baikal test
28+
make DAV_SERVER=davical test
29+
make DAV_SERVER=fastmail test
30+
make DAV_SERVER=icloud test
31+
32+
# Run specific test file:
33+
pytest tests/unit/sync/test_sync.py
34+
pytest tests/unit/sync/test_sync.py::test_name
35+
```
36+
37+
### Documentation
38+
```bash
39+
make docs # Build documentation
40+
make install-docs # Install docs dependencies
41+
```
42+
43+
### Release Process
44+
```bash
45+
make release # Build and upload release to PyPI
46+
```
47+
48+
## Architecture
49+
50+
### Core Components
51+
52+
1. **Storage Layer** (`vdirsyncer/storage/`)
53+
- `base.py`: Abstract base classes defining storage interface
54+
- `filesystem.py`: Local filesystem storage using vdir format
55+
- `dav.py`: CalDAV/CardDAV server storage
56+
- `google.py`: Google Calendar/Contacts storage
57+
- `http.py`: Read-only HTTP storage
58+
- `singlefile.py`: Single .ics/.vcf file storage
59+
- `memory.py`: In-memory storage for testing
60+
61+
2. **Sync Engine** (`vdirsyncer/sync/`)
62+
- `__init__.py`: Core sync algorithm based on OfflineIMAP
63+
- `status.py`: Tracks sync state between storages
64+
- `exceptions.py`: Sync-specific exceptions
65+
66+
3. **CLI** (`vdirsyncer/cli/`)
67+
- `config.py`: Configuration file parsing and validation
68+
- `discover.py`: Collection discovery implementation
69+
- `tasks.py`: Main CLI commands (sync, discover, etc.)
70+
- `utils.py`: CLI utilities and helpers
71+
72+
4. **Core Utilities**
73+
- `metasync.py`: Metadata synchronization
74+
- `repair.py`: Item repair functionality
75+
- `http.py`: HTTP session management with auth
76+
- `vobject.py`: vCard/iCalendar parsing utilities
77+
78+
### Key Design Patterns
79+
80+
- **Storage Interface**: All storage backends implement abstract methods from `Storage` base class
81+
- **Async Architecture**: Uses asyncio throughout for concurrent operations
82+
- **Item-based Sync**: Each calendar event or contact is an "item" with a unique href
83+
- **Conflict Resolution**: Configurable strategies (a wins, b wins, etc.)
84+
- **Collection Mapping**: Pairs of collections are configured and synced independently
85+
86+
### Configuration
87+
88+
Config files are in INI format with three section types:
89+
- `[general]`: Global settings
90+
- `[storage <name>]`: Storage backend definitions
91+
- `[pair <name>]`: Sync pair configuration linking two storages
92+
93+
## Code Style
94+
95+
- Python 3.7+ required
96+
- Uses Black formatter with 88 character line limit
97+
- Type hints encouraged but not required
98+
- Follow PEP 8 with Black's modifications
99+
100+
## Current Feature Branch
101+
102+
Currently on `storage-implicit-conflict-resolution` branch working on implicit collection creation during sync operations.

docs/config.rst

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,36 @@ Pair Section
128128

129129
The ``conflict_resolution`` parameter applies for these properties too.
130130

131+
.. _implicit_def:
132+
133+
- ``implicit``: Opt into implicitly creation and/or deleting collections. Example::
134+
135+
implicit = "create"
136+
137+
or
138+
139+
implicit = ["create", "delete"]
140+
141+
When a new collection is created on the source, and the value includes "create",
142+
new collections are created in the destination without asking questions. When
143+
the value includes "delete" and collections are removed from the source, they're
144+
automatically removed from the destination as well.
145+
146+
The value can be a string or an array of strings.
147+
148+
**Conflict Resolution**: When both "create" and "delete" are specified, there
149+
can be ambiguous situations where a collection exists on one side but not the
150+
other. In these cases, vdirsyncer uses the ``conflict_resolution`` parameter
151+
to determine the action:
152+
153+
- ``conflict_resolution = "a wins"``: Collections missing in A are created;
154+
collections missing in B are deleted from A.
155+
- ``conflict_resolution = "b wins"``: Collections missing in B are created;
156+
collections missing in A are deleted from B.
157+
158+
If both "create" and "delete" are specified but no ``conflict_resolution`` is
159+
set, vdirsyncer will raise an error when encountering ambiguous situations.
160+
131161
.. _storage_config:
132162

133163
Storage Section
@@ -381,8 +411,6 @@ Local
381411
#post_hook = null
382412
#pre_deletion_hook = null
383413
#fileignoreext = ".tmp"
384-
#implicit = "create"
385-
#implicit = ["create", "delete"]
386414

387415
Can be used with `khal <http://lostpackets.de/khal/>`_. See :doc:`vdir` for
388416
a more formal description of the format.
@@ -407,12 +435,7 @@ Local
407435
The command will be called with the path of the deleted file.
408436
:param fileeignoreext: The file extention to ignore. It is only useful
409437
if fileext is set to the empty string. The default is ``.tmp``.
410-
:param implicit: When a new collection is created on the source, and the
411-
value is "create", create the collection in the destination without
412-
asking questions. When the value is "delete" and a collection
413-
is removed on the source, remove it in the destination. The value
414-
can be a string or an array of strings. The deletion is implemented
415-
only for the "filesystem" storage.
438+
416439

417440
.. storage:: singlefile
418441

tests/storage/servers/davical/__init__.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,11 @@ async def inner(collection="test"):
3939
args = self.storage_class.create_collection(
4040
collection + str(uuid.uuid4()), **davical_args
4141
)
42-
s = self.storage_class(**args)
43-
if not list(s.list()):
44-
# See: https://stackoverflow.com/a/33984811
45-
request.addfinalizer(lambda x=s: x.session.request("DELETE", ""))
42+
storage = self.storage_class(**args)
43+
if not list(storage.list()):
44+
request.addfinalizer(
45+
lambda s=storage: s.session.request("DELETE", "")
46+
)
4647
return args
4748

4849
raise RuntimeError("Failed to find free collection.")

tests/system/cli/test_config.py

Lines changed: 154 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,8 @@ def test_read_config(read_config):
6262
"yesno": False,
6363
"number": 42,
6464
"instance_name": "bob_a",
65-
"implicit": [],
66-
},
67-
"bob_b": {
68-
"type": "carddav",
69-
"instance_name": "bob_b",
70-
"implicit": [],
7165
},
66+
"bob_b": {"type": "carddav", "instance_name": "bob_b"},
7267
}
7368

7469

@@ -227,3 +222,156 @@ def test_validate_collections_param():
227222
x([["c", None, "b"]])
228223
x([["c", "a", None]])
229224
x([["c", None, None]])
225+
226+
227+
def test_invalid_implicit_type(read_config):
228+
expected_message = "`implicit` parameter must be a list, string, or absent"
229+
with pytest.raises(exceptions.UserError) as excinfo:
230+
read_config(
231+
"""
232+
[general]
233+
status_path = "/tmp/status/"
234+
235+
[pair my_pair]
236+
a = "my_a"
237+
b = "my_b"
238+
collections = null
239+
implicit = 42
240+
241+
[storage my_a]
242+
type = "filesystem"
243+
path = "{base}/path_a/"
244+
fileext = ".txt"
245+
246+
[storage my_b]
247+
type = "filesystem"
248+
path = "{base}/path_b/"
249+
fileext = ".txt"
250+
"""
251+
)
252+
253+
assert expected_message in str(excinfo.value)
254+
255+
256+
def test_invalid_implicit_member(read_config):
257+
expected_message = "`implicit` parameter, position 0: must be one of"
258+
with pytest.raises(exceptions.UserError) as excinfo:
259+
read_config(
260+
"""
261+
[general]
262+
status_path = "/tmp/status/"
263+
264+
[pair my_pair]
265+
a = "my_a"
266+
b = "my_b"
267+
collections = null
268+
implicit = ["invalid"]
269+
270+
[storage my_a]
271+
type = "filesystem"
272+
path = "{base}/path_a/"
273+
fileext = ".txt"
274+
275+
[storage my_b]
276+
type = "filesystem"
277+
path = "{base}/path_b/"
278+
fileext = ".txt"
279+
"""
280+
)
281+
282+
assert expected_message in str(excinfo.value)
283+
284+
285+
def test_implicit_create_delete_with_conflict_resolution(read_config):
286+
"""Test that implicit create and delete works with conflict resolution."""
287+
# This should not raise an error
288+
errors, c = read_config(
289+
"""
290+
[general]
291+
status_path = "/tmp/status/"
292+
293+
[pair my_pair]
294+
a = "my_a"
295+
b = "my_b"
296+
collections = ["from a", "from b"]
297+
implicit = ["create", "delete"]
298+
conflict_resolution = "a wins"
299+
300+
[storage my_a]
301+
type = "filesystem"
302+
path = "{base}/path_a/"
303+
fileext = ".txt"
304+
305+
[storage my_b]
306+
type = "filesystem"
307+
path = "{base}/path_b/"
308+
fileext = ".txt"
309+
"""
310+
)
311+
312+
assert not errors
313+
pair = c.pairs["my_pair"]
314+
assert pair.implicit == ["create", "delete"]
315+
assert pair.conflict_resolution == "a wins"
316+
317+
318+
def test_implicit_create_delete_without_conflict_resolution(read_config):
319+
"""Test that implicit create and delete without conflict resolution still works in config parsing."""
320+
# Config parsing should succeed, but discovery should fail
321+
errors, c = read_config(
322+
"""
323+
[general]
324+
status_path = "/tmp/status/"
325+
326+
[pair my_pair]
327+
a = "my_a"
328+
b = "my_b"
329+
collections = ["from a", "from b"]
330+
implicit = ["create", "delete"]
331+
332+
[storage my_a]
333+
type = "filesystem"
334+
path = "{base}/path_a/"
335+
fileext = ".txt"
336+
337+
[storage my_b]
338+
type = "filesystem"
339+
path = "{base}/path_b/"
340+
fileext = ".txt"
341+
"""
342+
)
343+
344+
assert not errors
345+
pair = c.pairs["my_pair"]
346+
assert pair.implicit == ["create", "delete"]
347+
assert pair.conflict_resolution is None
348+
349+
350+
def test_implicit_create_only(read_config):
351+
"""Test that implicit create without delete works without conflict resolution."""
352+
errors, c = read_config(
353+
"""
354+
[general]
355+
status_path = "/tmp/status/"
356+
357+
[pair my_pair]
358+
a = "my_a"
359+
b = "my_b"
360+
collections = ["from a", "from b"]
361+
implicit = ["create"]
362+
363+
[storage my_a]
364+
type = "filesystem"
365+
path = "{base}/path_a/"
366+
fileext = ".txt"
367+
368+
[storage my_b]
369+
type = "filesystem"
370+
path = "{base}/path_b/"
371+
fileext = ".txt"
372+
"""
373+
)
374+
375+
assert not errors
376+
pair = c.pairs["my_pair"]
377+
assert pair.implicit == ["create"]

tests/system/utils/test_main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def test_get_storage_init_args():
2222
from vdirsyncer.storage.memory import MemoryStorage
2323

2424
all, required = utils.get_storage_init_args(MemoryStorage)
25-
assert all == {"fileext", "collection", "read_only", "instance_name", "implicit"}
25+
assert all == {"fileext", "collection", "read_only", "instance_name"}
2626
assert not required
2727

2828

0 commit comments

Comments
 (0)