Skip to content

Commit bfa92c0

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 6055664 commit bfa92c0

File tree

10 files changed

+478
-93
lines changed

10 files changed

+478
-93
lines changed

CHANGELOG.rst

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ Version 0.19.0
1919
- Add a new ``showconfig`` status. This prints *some* configuration values as
2020
JSON. This is intended to be used by external tools and helpers that interact
2121
with ``vdirsyncer``.
22+
- **BREAKING**: Improved ``implicit`` parameter behavior when both "create" and
23+
"delete" are specified. Previously, the behavior in ambiguous cases (when a
24+
collection exists on one side but not the other) was undefined. Now, the
25+
``conflict_resolution`` parameter is used to determine the action: "a wins"
26+
creates missing collections in B and deletes extra collections from A, while
27+
"b wins" does the opposite. If both "create" and "delete" are specified
28+
without ``conflict_resolution``, vdirsyncer will raise an error instead of
29+
making unpredictable choices.
2230
- Update TLS-related tests that were failing due to weak MDs. :gh:`903`
2331
- ``pytest-httpserver`` and ``trustme`` are now required for tests.
2432
- ``pytest-localserver`` is no longer required for tests.
@@ -38,12 +46,14 @@ Version 0.19.0
3846
- The ``plist`` for macOS has been dropped. It was broken and homebrew
3947
generates their own based on package metadata. macOS users are encouraged to
4048
use that as a reference.
49+
- Add ``implicit`` option to the :ref:`pair section <pair_config>`. It implicitly
50+
creates and deletes collections in the destinations, when new collections are
51+
created/deleted in the source. The deletion is implemented only for the
52+
"filesystem" storage. This respects your `conflict_resolution` configuration
53+
and shows an error when not configured or set to run a command.
54+
See :ref:`implicit docs <implicit_def>` for details.
4155

4256
.. _etesync-dav: https://github.com/etesync/etesync-dav
43-
- Add ``implicit`` option to storage section. It creates/deletes implicitly
44-
collections in the destinations, when new collections are created/deleted
45-
in the source. The deletion is implemented only for the "filesystem" storage.
46-
See :ref:`storage_config`.
4757

4858
Version 0.18.0
4959
==============

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
@@ -127,6 +127,36 @@ Pair Section
127127

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

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

132162
Storage Section
@@ -365,8 +395,6 @@ Local
365395
#encoding = "utf-8"
366396
#post_hook = null
367397
#fileignoreext = ".tmp"
368-
#implicit = "create"
369-
#implicit = ["create", "delete"]
370398

371399
Can be used with `khal <http://lostpackets.de/khal/>`_. See :doc:`vdir` for
372400
a more formal description of the format.
@@ -389,12 +417,7 @@ Local
389417
new/updated file.
390418
:param fileeignoreext: The file extention to ignore. It is only useful
391419
if fileext is set to the empty string. The default is ``.tmp``.
392-
:param implicit: When a new collection is created on the source, and the
393-
value is "create", create the collection in the destination without
394-
asking questions. When the value is "delete" and a collection
395-
is removed on the source, remove it in the destination. The value
396-
can be a string or an array of strings. The deletion is implemented
397-
only for the "filesystem" storage.
420+
398421

399422
.. storage:: singlefile
400423

tests/storage/servers/davical/__init__.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@ async def inner(collection="test"):
3737
args = self.storage_class.create_collection(
3838
collection + str(uuid.uuid4()), **davical_args
3939
)
40-
s = self.storage_class(**args)
41-
if not list(s.list()):
42-
request.addfinalizer(lambda: s.session.request("DELETE", ""))
40+
storage = self.storage_class(**args)
41+
if not list(storage.list()):
42+
request.addfinalizer(
43+
lambda s=storage: s.session.request("DELETE", "")
44+
)
4345
return args
4446

4547
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
@@ -60,13 +60,8 @@ def test_read_config(read_config):
6060
"yesno": False,
6161
"number": 42,
6262
"instance_name": "bob_a",
63-
"implicit": [],
64-
},
65-
"bob_b": {
66-
"type": "carddav",
67-
"instance_name": "bob_b",
68-
"implicit": [],
6963
},
64+
"bob_b": {"type": "carddav", "instance_name": "bob_b"},
7065
}
7166

7267

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

tests/system/utils/test_main.py

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

2222
all, required = utils.get_storage_init_args(MemoryStorage)
23-
assert all == {"fileext", "collection", "read_only", "instance_name", "implicit"}
23+
assert all == {"fileext", "collection", "read_only", "instance_name"}
2424
assert not required
2525

2626

0 commit comments

Comments
 (0)