Skip to content

Commit 535f8dc

Browse files
committed
refactor: use pure pimdir collection id
1 parent 05102ee commit 535f8dc

7 files changed

Lines changed: 113 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3131

3232
### Fixed
3333

34+
- Fixed the pimdir backend accepting a collection of any kind as an addressbook. A sync engine caches mail, calendars and contacts in one store, and only the listing narrowed them to `text/vcard`, so `card list -k imap/INBOX` printed a mailbox's messages as blank contacts and a create would have staged a vCard into it. An addressbook id now names an addressbook or nothing, and a wrong one is refused naming the addressbooks the account holds.
3435
- Fixed a card vanishing from the pimdir backend when another card of the same addressbook carried its `UID`. RFC 6352 requires that `UID` to be unique and servers hand over duplicates anyway, most often after a repeated import; the store keys the second copy apart now, so both cards list, read and act as ordinary cards under their own ids.
3536
- Fixed `card list` leaving the `TEL` column empty for any card writing its phone above its mail. The preview chained its three property reads, so a line that was not an `EMAIL` never reached the `TEL` read. Every backend was affected.
3637
- Fixed the pimdir backend linking a card it stages under `uid:<UID>` where the sync engine uses the bare `UID`, which would have stored the card twice and synced it as a duplicate contact. The derivations now come from io-pimdir's own conventions.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
cairn: delta
3+
change: pimdir-collection-id-is-the-addressbook
4+
---
5+
6+
## ADDED Requirements
7+
8+
### Requirement: A pimdir addressbook is its collection id
9+
The pimdir backend SHALL show and accept an address book as the store's collection id, verbatim: the collection `carddav/default` is the address book `carddav/default`. It SHALL NOT derive, strip or accept a shortened spelling, and no configuration SHALL offer one.
10+
11+
A sync engine binds a source's collections under a namespace, so an id carries one; the store is opaque to it, neither parsing nor validating an id (pimdir SPEC 9.2) and modelling hierarchy through `parent` rather than through a separator. Shortening is therefore a guess at the producer's convention, and one that makes a single address book answer to two spellings.
12+
13+
An id SHALL name an address book or name nothing: one store holds the collections of every kind a sync caches, so the kind SHALL narrow them at the one seam both the listing and the id check read, never at the listing alone. A kind-less collection counts, a sync having created one before kinds were declared.
14+
15+
An id naming no address book of the account SHALL be refused naming the ones it holds. Ids carry the sync engine's namespace and are not guessable, so an error asking for one that shows none leaves the user nothing to act on.
16+
17+
#### Scenario: A mailbox is not an address book
18+
- GIVEN a store a sync fills with mail, calendar and contact collections alike
19+
- WHEN a card command addresses a `message/rfc822` collection
20+
- THEN it is refused naming the address books the account holds, rather than listing the mailbox's messages as blank contacts
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
cairn: change
3+
id: pimdir-collection-id-is-the-addressbook
4+
status: landed
5+
created: 2026-08-28
6+
---
7+
8+
# A pimdir addressbook is its collection id
9+
10+
## Why
11+
12+
The sync engine binds a source's collections under a namespace, so an address book it caches is keyed `carddav/default` rather than `default`. Cardamum already addresses it by that id and shows it whole, which is the right shape: a collection id is opaque to the store, which neither parses nor validates it (pimdir SPEC 9.2) and models hierarchy through `parent` rather than through a separator, so shortening one would be a guess at the producer's convention rather than a lookup.
13+
14+
Himalaya learnt this the long way, having shipped a derived short name and a `pimdir.namespace` key to rescue the cases the derivation could not decide, and has just removed both. Cardamum never grew either, so the alignment is to say so before someone adds one, and to make the rule usable.
15+
16+
Usable is where two gaps showed. An id that names no collection is refused, but the refusal does not say what the store does hold, and an id carrying a namespace nobody can guess is one the user has to be shown. Worse, `known_collection` read the store's collections unfiltered while the listing filtered them by kind, so an id naming a collection of another kind passed the check: against a store Neverest fills with mail, calendars and contacts alike, `card list -k imap/INBOX` listed a mailbox's messages as blank contacts, and a create would have staged a vCard into it.
17+
18+
## What
19+
20+
The requirement is stated: a pimdir addressbook is its collection id, verbatim, with no derived spelling and no configuration offering one.
21+
22+
The kind filter moves into `collections()`, the one seam both the listing and the check read, so an id names an address book or it names nothing. The refusal then names the address books the account holds, so the id to type is in the error that asks for it.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
cairn: tasks
3+
change: pimdir-collection-id-is-the-addressbook
4+
---
5+
6+
- [x] State the requirement in the backends spec
7+
- [x] Filter by kind in `collections()`, so the check and the listing agree
8+
- [x] `known_collection` names the address books the account holds
9+
- [x] Verify against the real store: a mailbox id and an unknown id are both refused naming `carddav/default`
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
cairn: log
3+
date: 2026-08-28
4+
change: pimdir-collection-id-is-the-addressbook
5+
---
6+
7+
# A pimdir addressbook is its collection id
8+
9+
Neverest binds a source's collections under a namespace, so the address book it caches is keyed `carddav/default`. Cardamum already addressed it by that whole id, which is the right shape and was never written down: a collection id is opaque to the store, which neither parses nor validates it (pimdir SPEC 9.2) and models hierarchy through `parent` rather than through a separator, so shortening one is a guess at the producer's convention rather than a lookup. Himalaya had shipped such a guess, with a `pimdir.namespace` key to rescue the stores it could not decide, and removed both the same day. Stating the rule here is what stops it being added.
10+
11+
Writing it down found the two things that made the rule hard to live with.
12+
13+
## What landed
14+
15+
**The kind narrows at one seam, in src/pimdir/backend.rs.** `collections()` returned the account's collections of every kind, and only `list_addressbooks` filtered them down to `text/vcard`. `known_collection` read the same unfiltered list, so an id naming a collection of another kind passed the check. Against a store Neverest fills with mail, calendars and contacts alike, `card list -k imap/INBOX` listed a mailbox's messages as blank contacts, and a create would have staged a vCard into it. The filter moved into `collections()`, which both callers read, so an id names an address book or it names nothing.
16+
17+
**The refusal names what the account holds.** `Addressbook \`default\` not found` became `Addressbook \`default\` not found; this account holds: carddav/default`. An id carrying a namespace is not guessable, so an error asking for one has to show the choices, and the same message now answers a wrong-kind id and a mistyped one.
18+
19+
## Capabilities moved
20+
21+
- backends: added *A pimdir addressbook is its collection id*
22+
23+
## Verification
24+
25+
Built clean and run against the live Neverest store at `~/.local/state/neverest/posteo`, which holds eighteen collections across three kinds under one account. Before the fix, `card list -k imap/INBOX` printed sixteen blank contacts; after it, that id and `default` are both refused naming `carddav/default`, `card list -k carddav/default` lists the contacts, and `addressbook list` is unchanged.

cairn/spec/backends.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ vdir and pimdir SHALL adapt io-vdir and io-pimdir. vdir stores each addressbook
3434
### Requirement: A cache renames nothing it cannot push
3535
The pimdir backend SHALL refuse every `addressbook update` field. Its collection row (id, display name, description, colour) is written by the sync from the server, and the backend stages item actions only, so any local edit of it is a change no sync would carry. A missing body SHALL fall back to the summary preview in a listing, as an unhydrated card does, rather than render as a blank row.
3636

37+
### Requirement: A pimdir addressbook is its collection id
38+
The pimdir backend SHALL show and accept an address book as the store's collection id, verbatim: the collection `carddav/default` is the address book `carddav/default`. It SHALL NOT derive, strip or accept a shortened spelling, and no configuration SHALL offer one.
39+
40+
A sync engine binds a source's collections under a namespace, so an id carries one; the store is opaque to it, neither parsing nor validating an id (pimdir SPEC 9.2) and modelling hierarchy through `parent` rather than through a separator. Shortening is therefore a guess at the producer's convention, and one that makes a single address book answer to two spellings.
41+
42+
An id SHALL name an address book or name nothing: one store holds the collections of every kind a sync caches, so the kind SHALL narrow them at the one seam both the listing and the id check read, never at the listing alone. A kind-less collection counts, a sync having created one before kinds were declared.
43+
44+
An id naming no address book of the account SHALL be refused naming the ones it holds. Ids carry the sync engine's namespace and are not guessable, so an error asking for one that shows none leaves the user nothing to act on.
45+
3746
### Requirement: pimdir is a cache, not a server
3847
The pimdir backend SHALL treat the store as a possibly-partial cache. `get_card` on a card whose body is not local (`level < Full`, no stored object) SHALL report a clear "body not fetched" state, the cue to sync, rather than a data-loss error. The card still lists: `list_cards` SHALL project the stored `v: 1` summary into a minimal preview vCard (`UID`, `FN`, `EMAIL`) so a contact list reads correctly before a full sync, while `get_card` refuses outright so a preview can never be mistaken for the document of record.
3948

src/pimdir/backend.rs

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ impl PimdirBackend {
5858
let mut addressbooks: Vec<Addressbook> = self
5959
.collections()?
6060
.into_iter()
61-
.filter(|collection| collection.kind.is_empty() || collection.kind == CARD_KIND)
6261
.map(|collection| Addressbook {
6362
name: if collection.name.is_empty() {
6463
collection.id.clone()
@@ -229,8 +228,13 @@ impl PimdirBackend {
229228
Ok(())
230229
}
231230

232-
/// The store's collections, narrowed to the configured account when the
233-
/// store groups several (pimdir SPEC §9.2).
231+
/// The store's address book collections, narrowed to the configured
232+
/// account when the store groups several (pimdir SPEC §9.2).
233+
///
234+
/// One store holds the collections of every kind a sync caches, so the
235+
/// kind is what separates an address book from a mailbox or a calendar.
236+
/// A kind-less collection counts: a sync that created one before kinds
237+
/// were declared left the column empty.
234238
fn collections(&self) -> Result<Vec<PimdirCollection>> {
235239
let collections = match self.inner.account.as_deref() {
236240
Some(account) => self
@@ -240,7 +244,10 @@ impl PimdirBackend {
240244
None => self.inner.reader.list_collections()?,
241245
};
242246

243-
Ok(collections)
247+
Ok(collections
248+
.into_iter()
249+
.filter(|collection| collection.kind.is_empty() || collection.kind == CARD_KIND)
250+
.collect())
244251
}
245252

246253
/// Pulls every live item of a collection by keyset paging, in the
@@ -309,23 +316,32 @@ impl PimdirBackend {
309316
})
310317
}
311318

312-
/// Fails unless `collection` is a collection the store knows.
319+
/// Fails unless `collection` is a collection the store knows, naming the
320+
/// ones it does hold.
313321
///
314322
/// The store's read seam answers an unknown collection with an empty page
315323
/// and its queue accepts an action for any name, so without this a typo in
316324
/// `-k` would read as an empty addressbook and stage into one nothing will
317-
/// ever apply.
325+
/// ever apply. An addressbook is its collection id, which carries the sync
326+
/// engine's namespace and is not guessable, so the refusal shows the ids to
327+
/// choose from.
318328
fn known_collection(&self, collection: &str) -> Result<()> {
319-
let known = self
329+
let mut ids: Vec<String> = self
320330
.collections()?
321331
.into_iter()
322-
.any(|candidate| candidate.id == collection);
332+
.map(|candidate| candidate.id)
333+
.collect();
323334

324-
if !known {
325-
bail!("Addressbook `{collection}` not found");
335+
if ids.iter().any(|id| id == collection) {
336+
return Ok(());
326337
}
327338

328-
Ok(())
339+
ids.sort();
340+
341+
bail!(
342+
"Addressbook `{collection}` not found; this account holds: {}",
343+
ids.join(", "),
344+
)
329345
}
330346

331347
/// The stored item behind a public card id, or a clear miss.

0 commit comments

Comments
 (0)