import: keep a plan as YAML a person can edit
The plan is YAML now, and a card is a form: a name, the phones, the groups it goes in, so the file a person edits before execute is the contact the host takes in. The builder fills those fields from FN, N, and one exact cell TEL, and every other line is refused: a field holds less than a line does, so a name it would shorten is news. Assisted-by: Claude Opus 5 via Claude Code Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Issue: rtqvmzvp
diff --git a/AGENTS.md b/AGENTS.md
index 516eb68..02c9090 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -26,8 +26,8 @@ rake dev # Dev server, seeded from test/fixtures/cards into a
# (needs fd and entr)
rake import:macos:plan # Plan importing this Mac's iCloud contacts
# (LIMIT=n; reads Contacts, so leave it to the user)
-rake import:execute # Land an import plan on a host
- # (PLAN=dir HOST=https://contacts)
+rake import:execute # Land the newest plan in data/imports on a host
+ # (HOST=https://contacts; PLAN=dir for another)
rake profile:install # Download carddav.mobileconfig from the app and
# stage it for approval
```
diff --git a/docs/plans/2026-09-16-importing-from-macos.md b/docs/plans/2026-09-16-importing-from-macos.md
index 3bcf1e5..b25fe31 100644
--- a/docs/plans/2026-09-16-importing-from-macos.md
+++ b/docs/plans/2026-09-16-importing-from-macos.md
@@ -13,15 +13,15 @@ holds every contact in full:
| Path | Holds |
|---|---|
-| `plan.json` | The source, when it was built, the group, and each contact's minted id beside its source id; then the host, the group's id, and each contact's status |
-| `cards/<id>.vcf` | The bytes `execute` PUTs, byte for byte |
+| `plan.yml` | The source, when it was built, the group, and each contact's minted id beside its source id; then the host and each contact's status |
+| `cards/<id>.yml` | The contact's fields and the groups it belongs to, for a person to edit before `execute` |
| `backups/<id>/` | The original, in whatever files the source reads it as |
-The builder writes the cards and backups once and nothing rewrites them.
-A plan has no open questions in it: every card is final when the plan is
-written, so `execute` runs it rather than finishing it.
+The builder writes the cards and backups once, and no code rewrites
+them. A person can: a card file is the plan's one open question, read as
+it stands when `execute` carries it.
-`plan.json` also records how far `execute` and `remove` have carried the
+`plan.yml` also records how far `execute` and `remove` have carried the
plan, saved after every step. A contact's status goes from none to
`landed`, `joined`, and `removed`. Each save writes a new file and renames
it over the old one, so a run that dies mid-save leaves the plan as the
@@ -52,12 +52,17 @@ original.
## Execute is the same for every source
-`rake import:execute PLAN=dir HOST=host` does the following, recording
+`rake import:execute HOST=host` lands the newest plan in `data/imports`,
+the one just planned, and `PLAN=dir` names another. It does the
+following, recording
each step in the plan:
-1. PUT each card to `/dav/addressbook/<id>.vcf` with `If-None-Match: *`.
-2. Create the plan's group, `import-<timestamp>`, through `POST /groups`.
-3. Join each card to it through `POST /contacts/<id>/groups`.
+1. Read every card not yet joined, and stop before any write if one
+ will not read.
+2. PUT each card to `/dav/addressbook/<id>.vcf` with `If-None-Match: *`.
+3. Put each card in the groups its file names through
+ `POST /contacts/<id>/groups`, creating any that are missing through
+ `POST /groups`.
A rerun skips every step the plan records. The plan alone is not
enough, because a run can die after a write lands and before the plan
@@ -69,10 +74,43 @@ first keeps a rerun from collecting refusals, each of which warns Sentry
- A bare 412 to the PUT is `If-None-Match` failing, and at 48 bits a
minted id is taken only by this plan's own earlier PUT, so the card
counts as landed. A 412 with a body is the card refused.
-- The group is looked up by name in `GET /api/groups`, every group's id
- and name as JSON, before it is created. A name that is already there
- is this plan's, since it carries the plan's timestamp.
+- A group is looked up by name in `GET /api/groups`, every group's id
+ and name as JSON, before it is created, so a rerun reuses the group an
+ earlier run made.
- Joining a group twice is joining once (`Store#add_member`).
+- A PUT that creates a card makes it a member of `sync:*`
+ (`2026-09-15-client-creates-join-everyone.md`). The request that sets
+ a card's groups therefore lists `sync:*` among the groups it was
+ already in, so a card whose file does not name `sync:*` is taken back
+ out of it.
+
+## A card is a form filled in
+
+A card file holds what the web editor would: a first and last name and
+a list of phone numbers, plus the names of the groups it belongs to. The plan
+writes `sync:*` and its own `import-<timestamp>` group into every card,
+ahead of any the builder chose.
+
+```yaml
+first: Ada
+last: Lovelace
+phones:
+- "+12532189075"
+groups:
+- sync:*
+- import-20260916T180412Z
+```
+
+`Import::Card` turns the fields into a contact through the web's own
+code: `CardForm.new_card`, then `CardForm.contact_card` with each phone
+as an add row. A phone's types do not survive, because the form has no
+field for them, so a number lands as a bare `TEL`.
+
+A file that will not read as a card is refused, naming the file: a
+missing or unknown key, a name with neither half, or a value YAML reads
+as something other than text. An unquoted `no` is false and an unquoted
+`+12532189075` is a number, and either would otherwise import as
+something no one typed.
`HOST` is a base URL, such as `https://contacts` or the dev server's
`http://localhost:9292`.
@@ -124,14 +162,16 @@ with image data in base64, and the AppleScript note.
## The builder knows only what it was taught
-The macOS builder turns each line into a card line through a `case` on
-the property name, whose `else` records the line as unknown. The envelope
-is not carried but written around the lines, so the minted `UID` goes
-inside it; a `VERSION` other than 3.0 is unknown. The note and the image
+The macOS builder decides each line through a `case` on the property
+name, whose `else` records the line as unknown, then fills the card's
+fields from the lines it kept. The envelope is dropped, since the web's
+create writes its own; a `VERSION` other than 3.0 is unknown. An Apple `PRODID` is
+dropped, since Contacts writes its own on every save. The note and the image
data, which the vCard leaves out, are unknown until a branch takes them
too. Each branch
-names the parameters it accepts, and a parameter outside that list is
-unknown too. Once every contact in the batch is read, any unknown fails
+names the forms it accepts, a form being the group, name, and parameters
+spelled exactly as the source wrote them, and a line in any other form is
+unknown under that form. Once every contact in the batch is read, any unknown fails
the build with every unknown name, its count, and a few distinct values
it held, each beside its source id and cut short when long, and
no plan is written. A small batch needs only the branches its own
@@ -140,8 +180,15 @@ contacts use.
The `case` starts empty. A property earns a branch when a build refuses
it, and the branch is written with that property in view, so nothing is
imported on the strength of a guess about what it holds. Parameter
-values such as `TYPE=IPHONE` are not checked: the server passes `TYPE`
-through, and Apple wrote them.
+values are part of the form, so `TYPE=IPHONE` is refused until a branch
+names it. A property whose value has a shape can check it too: `TEL`
+takes only a `+` and digits.
+
+A field holds less than a line can, so a line that would lose something
+on the way into one is unknown too. The name comes from the one `N`,
+and is refused when `N` holds more than a family and given name, or when
+the one `FN` is not those two joined with a space, the way the web
+editor writes it.
## Removing the originals
diff --git a/lib/pro_tacts/import/card.rb b/lib/pro_tacts/import/card.rb
new file mode 100644
index 0000000..0006079
--- /dev/null
+++ b/lib/pro_tacts/import/card.rb
@@ -0,0 +1,67 @@
+require "yaml"
+
+require "pro_tacts/admin/card_form"
+require "pro_tacts/contact"
+
+module ProTacts
+ module Import
+ # One contact in a plan, as the file a person edits before execute:
+ # the fields the web editor has, and the groups to join
+ # (docs/plans/2026-09-16-importing-from-macos.md, "A card is a form
+ # filled in"). Signed in sig/pro_tacts/import.rbs, being a Data class.
+ # @rbs skip
+ Card = Data.define(:first, :last, :phones, :groups)
+
+ # Reopened for the reason VCard::Parser::Property is.
+ class Card
+ # A card file that will not read as one.
+ class Invalid < StandardError; end
+
+ KEYS = %w[first last phones groups].freeze #: Array[String]
+
+ # Every problem is refused rather than skipped: a key this does not
+ # know, or a value YAML read as something other than text (an
+ # unquoted `no` is false), is an edit that would otherwise vanish.
+ #: (Pathname path) -> Card
+ def self.read(path)
+ document = YAML.safe_load_file(path)
+ invalid = ->(why) { raise Invalid, "#{path}: #{why}" }
+ invalid.("is not a mapping") unless document.is_a?(Hash)
+
+ missing = KEYS - document.keys
+ invalid.("has no #{missing.join(", ")}") unless missing.empty?
+ unknown = document.keys - KEYS
+ invalid.("has #{unknown.join(", ")}, which no field takes") unless unknown.empty?
+
+ first, last, phones, groups = document.values_at(*KEYS)
+ invalid.("first and last must be text; quote them") unless first.is_a?(String) && last.is_a?(String)
+ invalid.("needs a first or last name") if first.strip.empty? && last.strip.empty?
+ invalid.("phones must be a list of quoted numbers") unless texts?(phones)
+ invalid.("groups must be a list of names") unless texts?(groups) && groups.none? { it.strip.empty? }
+
+ new(first:, last:, phones:, groups:)
+ end
+
+ #: (untyped value) -> bool
+ def self.texts?(value)
+ value.is_a?(Array) && value.all?(String)
+ end
+
+ private_class_method :texts?
+
+ # The contact the web create and then the editor's add rows would
+ # make of these fields.
+ #: (String id) -> Contact
+ def contact(id)
+ created = Contact.new(id:, stored: Admin::CardForm.new_card(id, first, last), birthday: nil, inherited: [])
+ stored = Admin::CardForm.contact_card(created, first, last, {"new_phone" => phones})
+ Contact.new(id:, stored:, birthday: nil, inherited: [])
+ end
+
+ #: () -> Hash[String, untyped]
+ def document
+ {"first" => first, "last" => last, "phones" => phones, "groups" => groups}
+ end
+ end
+ end
+end
diff --git a/lib/pro_tacts/import/execute.rb b/lib/pro_tacts/import/execute.rb
index c35e5db..74ee029 100644
--- a/lib/pro_tacts/import/execute.rb
+++ b/lib/pro_tacts/import/execute.rb
@@ -13,6 +13,7 @@ module ProTacts
# @rbs @plan: Plan
# @rbs @host: String
# @rbs @client: _Client
+ # @rbs @group_ids: Hash[String, String]
class Failed < StandardError; end
@@ -31,6 +32,7 @@ module ProTacts
@plan = plan
@host = host
@client = client
+ @group_ids = {}
end
#: () -> void
@@ -39,13 +41,17 @@ module ProTacts
if landed_on && landed_on != @host
raise Failed, "#{@plan.dir} is landing on #{landed_on}; refusing to land it on #{@host} too"
end
+ # Every card still to carry is read before the first write, so an
+ # edit that will not read stops the run with nothing landed.
+ cards = @plan.contacts.reject { it.status == "joined" }.to_h {
+ [it.id, @plan.card(it.id)] #: [String, Card]
+ }
# Before the first write, so a run that dies after one still ties
# the plan to this host.
@plan.host = @host unless landed_on
- @plan.contacts.each { land(it.id) if it.status.nil? }
- group_id = @plan.group_id || make_group
- @plan.contacts.each { join(it.id, group_id) if it.status == "landed" }
+ @plan.contacts.each { land(it.id, cards.fetch(it.id)) if it.status.nil? }
+ @plan.contacts.each { join(it.id, cards.fetch(it.id)) if it.status == "landed" }
end
private
@@ -54,12 +60,12 @@ module ProTacts
# the card is on the host already: an earlier run's PUT landed and
# died before the plan recorded it. A 412 with a body is the card
# refused.
- #: (String id) -> void
- def land(id)
+ #: (String id, Card card) -> void
+ def land(id, card)
response = @client.call(
"PUT", "/dav/addressbook/#{id}.vcf",
headers: {"Content-Type" => "text/vcard; charset=utf-8", "If-None-Match" => "*"},
- body: @plan.card(id)
+ body: card.contact(id).vcard.to_s
)
unless [201, 204].include?(response.status) || response.status == 412 && response.body.empty?
raise Failed, "PUT of #{id} answered #{response.status}: #{response.body}"
@@ -68,37 +74,40 @@ module ProTacts
@plan.record(id, "landed")
end
+ # The card's groups, and out of `sync:*` unless it is one of them:
+ # the PUT that created the card put it there.
+ #: (String id, Card card) -> void
+ def join(id, card)
+ form = card.groups.uniq.map { ["groups[]", group_id(it)] } #: Array[[String, String]]
+ form << ["was[]", group_id(Plan::EVERYONE)]
+ response = post("/contacts/#{id}/groups", form)
+ raise Failed, "joining #{id} to #{card.groups.join(", ")} answered #{response.status}" unless response.status == 303
+
+ @plan.record(id, "joined")
+ end
+
# Looked up before it is made, so a run that died after making it
# joins the one it made.
- #: () -> String
- def make_group
- id = listed_group_id
- unless id
- response = post("/groups", [["name", @plan.group]])
- raise Failed, "creating group #{@plan.group} answered #{response.status}" unless response.status == 303
-
- id = listed_group_id or raise Failed, "group #{@plan.group} was created and is not listed"
- end
+ #: (String name) -> String
+ def group_id(name)
+ @group_ids[name] ||= listed_group_id(name) || make_group(name)
+ end
- @plan.group_id = id
- id
+ #: (String name) -> String
+ def make_group(name)
+ response = post("/groups", [["name", name]])
+ raise Failed, "creating group #{name} answered #{response.status}" unless response.status == 303
+
+ listed_group_id(name) or raise Failed, "group #{name} was created and is not listed"
end
- #: () -> String?
- def listed_group_id
+ #: (String name) -> String?
+ def listed_group_id(name)
response = @client.call("GET", "/api/groups")
raise Failed, "listing groups answered #{response.status}" unless response.status == 200
groups = JSON.parse(response.body) #: Array[Hash[String, String?]]
- groups.find { it.fetch("name") == @plan.group }&.fetch("id")
- end
-
- #: (String id, String group_id) -> void
- def join(id, group_id)
- response = post("/contacts/#{id}/groups", [["groups[]", group_id]])
- raise Failed, "joining #{id} to #{@plan.group} answered #{response.status}" unless response.status == 303
-
- @plan.record(id, "joined")
+ groups.find { it.fetch("name") == name }&.fetch("id")
end
#: (String path, Array[[String, String]] form) -> Response
diff --git a/lib/pro_tacts/import/macos.rb b/lib/pro_tacts/import/macos.rb
index 701f6cc..565ac60 100644
--- a/lib/pro_tacts/import/macos.rb
+++ b/lib/pro_tacts/import/macos.rb
@@ -3,6 +3,7 @@ require "open3"
require "pathname"
require "pro_tacts/change_id"
+require "pro_tacts/import/card"
require "pro_tacts/import/plan"
require "pro_tacts/vcard/parser"
@@ -89,7 +90,9 @@ module ProTacts
note = record.fetch("note") #: String?
contact = record.fetch("contact") #: Hash[String, untyped]
- body = VCard::Parser.lines(vcard).filter_map { line(it, source_id, unknown) }
+ carried = VCard::Parser.lines(vcard).select { carried?(it, source_id, unknown) }
+ first, last = name(carried, source_id, unknown)
+ phones = carried.filter_map { it.property&.then { |property| property.text if property.name.casecmp?("TEL") } }
# Beyond the vCard: what the serializer leaves out.
unknown.add("note", source_id, note) if note
image = contact.fetch("imageData") #: String?
@@ -98,38 +101,95 @@ module ProTacts
backup = {"original.vcf" => vcard, "contact.json" => JSON.pretty_generate(contact)}
backup["note.txt"] = note if note
- Plan::Entry.new(
- id:, source_id:,
- card: "BEGIN:VCARD\r\nVERSION:3.0\r\n#{body.join}UID:#{id}\r\nEND:VCARD\r\n",
- backup:
- )
+ Plan::Entry.new(id:, source_id:, card: Card.new(first:, last:, phones:, groups: []), backup:)
end
- # A line of the source card as a line of the imported one, or nil for
- # one it does not carry. The envelope is written around the lines
- # rather than carried, so the card's UID can go inside it.
- #: (VCard::Parser::Line line, String source_id, Unknown unknown) -> String?
- def self.line(line, source_id, unknown)
+ # Whether a line of the source card is carried into the card's
+ # fields; one that is not is recorded unknown or, for the envelope
+ # and PRODID, dropped.
+ #: (VCard::Parser::Line line, String source_id, Unknown unknown) -> bool
+ def self.carried?(line, source_id, unknown)
property = line.property
- example = VCard::Parser.unfold(line.verbatim).chomp
+ example = example(line)
if property.nil?
unknown.add("unreadable line", source_id, example) unless example.strip.empty?
- return
+ return false
end
- case property.name.upcase
+ name = property.name.upcase
+ case name
when "BEGIN", "END"
- nil
+ false
when "VERSION"
unknown.add("VERSION:#{property.value}", source_id, example) unless property.value == "3.0"
- nil
+ false
+ when "PRODID"
+ # Contacts writes its own on every save; the source's stays in the backup.
+ form = form(property, example)
+ unknown.add(form, source_id, example) unless form == "PRODID"
+ unknown.add("PRODID value", source_id, example) unless property.value.start_with?("-//Apple Inc.//")
+ false
+ when "FN", "N", "TEL"
+ refused = [] #: Array[String]
+ form = form(property, example)
+ refused << form unless FORMS.fetch(name).include?(form)
+ refused << "TEL value" if name == "TEL" && !property.value.match?(PHONE)
+ refused.each { unknown.add(it, source_id, example) }
+ refused.empty?
else
- unknown.add(property.name.upcase, source_id, example)
- nil
+ unknown.add(name, source_id, example)
+ false
end
end
- private_class_method :entry, :line
+ # The first and last name, from the one N, when the one FN is those
+ # two joined as the web editor joins them: a card holding more of a
+ # name than that would lose it.
+ #: (Array[VCard::Parser::Line] lines, String source_id, Unknown unknown) -> [String, String]
+ def self.name(lines, source_id, unknown)
+ ns = lines.select { it.names?("N") }
+ fns = lines.select { it.names?("FN") }
+ unknown.add("N lines: #{ns.size}", source_id, ns.map { example(it) }.join(" / ")) unless ns.size == 1
+ unknown.add("FN lines: #{fns.size}", source_id, fns.map { example(it) }.join(" / ")) unless fns.size == 1
+
+ n = ns.first
+ family, given, *rest = n&.property&.components || []
+ first = given.to_s
+ last = family.to_s
+ unknown.add("N beyond first and last", source_id, example(n)) if n && rest.any? { !it.empty? }
+ unknown.add("no name", source_id, n ? example(n) : "") if n && first.empty? && last.empty?
+
+ fn = fns.first
+ if fn && fn.property&.text != [first, last].reject(&:empty?).join(" ")
+ unknown.add("FN other than first and last", source_id, example(fn))
+ end
+
+ [first, last]
+ end
+
+ # The spellings each carried property is taken in, group and
+ # parameters included, exactly as the source wrote them.
+ FORMS = {
+ "FN" => ["FN"],
+ "N" => ["N"],
+ "TEL" => ["TEL;type=CELL;type=VOICE;type=pref"]
+ }.freeze #: Hash[String, Array[String]]
+
+ PHONE = /\A\+[0-9]+\z/ #: Regexp
+
+ # A line unfolded, without its terminator.
+ #: (VCard::Parser::Line line) -> String
+ def self.example(line)
+ VCard::Parser.unfold(line.verbatim).chomp
+ end
+
+ # A line as the source spelled it, less its value.
+ #: (VCard::Parser::Property property, String example) -> String
+ def self.form(property, example)
+ example.delete_suffix(":#{property.value}")
+ end
+
+ private_class_method :entry, :carried?, :name, :example, :form
end
end
end
diff --git a/lib/pro_tacts/import/plan.rb b/lib/pro_tacts/import/plan.rb
index 2477c26..d4b5fb4 100644
--- a/lib/pro_tacts/import/plan.rb
+++ b/lib/pro_tacts/import/plan.rb
@@ -1,7 +1,9 @@
require "fileutils"
-require "json"
require "pathname"
require "time"
+require "yaml"
+
+require "pro_tacts/import/card"
module ProTacts
module Import
@@ -12,9 +14,9 @@ module ProTacts
# @rbs @dir: Pathname
# @rbs @manifest: Hash[String, untyped]
- # What a builder hands over for one contact: the card execute PUTs
- # and the original's files, by name. Signed in
- # sig/pro_tacts/import.rbs, being a Data class.
+ # What a builder hands over for one contact: its card, which the
+ # plan's own groups are added to, and the original's files, by
+ # name. Signed in sig/pro_tacts/import.rbs, being a Data class.
# @rbs skip
Entry = Data.define(:id, :source_id, :card, :backup)
@@ -25,6 +27,9 @@ module ProTacts
FORMAT = 1 #: Integer
+ # Store::EVERYONE, spelled out so a plan loads no database.
+ EVERYONE = "sync:*" #: String
+
#: (Pathname dir, source: String, created_at: Time, entries: Array[Entry]) -> Plan
def self.write(dir, source:, created_at:, entries:)
FileUtils.mkdir_p(dir.dirname)
@@ -32,9 +37,12 @@ module ProTacts
# another.
Dir.mkdir(dir)
+ group = "import-#{created_at.utc.strftime("%Y%m%dT%H%M%SZ")}"
entries.each do |entry|
(dir / "cards").mkpath
- (dir / "cards/#{entry.id}.vcf").binwrite(entry.card)
+ card = entry.card
+ card = Card.new(first: card.first, last: card.last, phones: card.phones, groups: [EVERYONE, group, *card.groups])
+ (dir / "cards/#{entry.id}.yml").write(YAML.dump(card.document))
backup = dir / "backups" / entry.id
backup.mkpath
entry.backup.each { |name, bytes| (backup / name).binwrite(bytes) }
@@ -44,9 +52,8 @@ module ProTacts
"format" => FORMAT,
"source" => source,
"created_at" => created_at.utc.iso8601,
- "group" => "import-#{created_at.utc.strftime("%Y%m%dT%H%M%SZ")}",
+ "group" => group,
"host" => nil,
- "group_id" => nil,
"contacts" => entries.map { {"id" => it.id, "source_id" => it.source_id, "status" => nil} }
})
plan.save
@@ -55,7 +62,7 @@ module ProTacts
#: (Pathname dir) -> Plan
def self.read(dir)
- new(dir, JSON.parse((dir / "plan.json").read))
+ new(dir, YAML.safe_load_file(dir / "plan.yml"))
end
attr_reader :dir #: Pathname
@@ -88,15 +95,6 @@ module ProTacts
save
end
- #: () -> String?
- def group_id = @manifest.fetch("group_id")
-
- #: (String id) -> void
- def group_id=(id)
- @manifest["group_id"] = id
- save
- end
-
#: () -> Array[Contact]
def contacts
@manifest.fetch("contacts").map { Contact.new(id: it.fetch("id"), source_id: it.fetch("source_id"), status: it.fetch("status")) }
@@ -108,21 +106,22 @@ module ProTacts
save
end
- #: (String id) -> String
+ # The card as its file now reads, edits included.
+ #: (String id) -> Card
def card(id)
- (dir / "cards/#{id}.vcf").binread
+ Card.read(dir / "cards/#{id}.yml")
end
# Every step is saved as it is taken, and replaced whole by a rename
# so a run that dies mid-write leaves the plan as it was before.
#: () -> void
def save
- written = dir / "plan.json.tmp"
+ written = dir / "plan.yml.tmp"
written.open("w") do |file|
- file.write(JSON.pretty_generate(@manifest))
+ file.write(YAML.dump(@manifest))
file.fsync
end
- written.rename(dir / "plan.json")
+ written.rename(dir / "plan.yml")
end
end
end
diff --git a/sig/pro_tacts/import.rbs b/sig/pro_tacts/import.rbs
index fcf589d..0df6dce 100644
--- a/sig/pro_tacts/import.rbs
+++ b/sig/pro_tacts/import.rbs
@@ -1,18 +1,27 @@
-# Import::Plan::Entry, ::Contact, and Import::Execute::Response are Data
+# Import::Card, Import::Plan::Entry, ::Contact, and Import::Execute::Response are Data
# classes, whose members the inline parser cannot read (see
# sig/pro_tacts/contact.rbs). Each is marked `@rbs skip` where it is
# defined and signed here. _Client is an interface, which inline syntax
# has no way to declare.
module ProTacts
module Import
+ class Card
+ attr_reader first: String
+ attr_reader last: String
+ attr_reader phones: Array[String]
+ attr_reader groups: Array[String]
+
+ def self.new: (first: String, last: String, phones: Array[String], groups: Array[String]) -> instance
+ end
+
class Plan
class Entry
attr_reader id: String
attr_reader source_id: String
- attr_reader card: String
+ attr_reader card: Card
attr_reader backup: Hash[String, String]
- def self.new: (id: String, source_id: String, card: String, backup: Hash[String, String]) -> instance
+ def self.new: (id: String, source_id: String, card: Card, backup: Hash[String, String]) -> instance
end
class Contact
diff --git a/tasks/import.rake b/tasks/import.rake
index 1e6d58d..d4b45ee 100644
--- a/tasks/import.rake
+++ b/tasks/import.rake
@@ -14,13 +14,18 @@ namespace :import do
records = ProTacts::Import::Macos.read(limit:)
plan = ProTacts::Import::Macos.plan(dir, records, created_at:)
+ plan.contacts.each do |contact|
+ card = plan.card(contact.id)
+ name = [card.first, card.last].reject(&:empty?).join(" ")
+ puts "#{contact.id} #{name}#{card.phones.map { " #{it}" }.join}"
+ end
puts "#{plan.contacts.size} contacts planned in #{dir}"
rescue ProTacts::Import::Macos::Unknown => error
abort error.message
end
end
- desc "Land the plan in PLAN on the pro-tacts at HOST, a base URL such as https://contacts"
+ desc "Land the newest plan in data/imports, or the one in PLAN, on the pro-tacts at HOST, a base URL such as https://contacts"
task :execute do
require "net/http"
require "pathname"
@@ -29,13 +34,30 @@ namespace :import do
require "pro_tacts/import/http_client"
require "pro_tacts/import/plan"
- plan = ProTacts::Import::Plan.read(Pathname.new(ENV.fetch("PLAN")))
- host = ENV.fetch("HOST")
- uri = URI.parse(host)
+ # The plan task writes under data/imports, and names each plan for the
+ # minute it was built, so the newest is the one just planned. A PLAN
+ # that is not a directory is read as a plan's name under there, the
+ # way the directories there are named.
+ named = ENV.fetch("PLAN", nil)&.then { Pathname.new(it) }
+ named = Pathname.new("data/imports") / named if named && !named.directory?
+ dir = named ||
+ Pathname.glob("data/imports/*/plan.yml").map(&:dirname).max_by { it.basename.to_s } ||
+ abort("no plan in data/imports: run rake import:macos:plan, or name one in PLAN")
+ abort("#{dir} holds no plan.yml") unless (dir / "plan.yml").file?
+
+ plan = ProTacts::Import::Plan.read(dir)
+ # A bare hostname is the base URL of a server that serves HTTPS, which
+ # every deployment does; the scheme is spelled out here so the host
+ # the plan records is the one a second run is compared against.
+ uri = URI.parse(ENV.fetch("HOST"))
+ uri = URI.parse("https://#{uri}") if uri.scheme.nil?
+ host = uri.to_s
+ puts "landing #{dir} on #{host}"
Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |http|
ProTacts::Import::Execute.call(plan, host:, client: ProTacts::Import::HttpClient.new(http))
end
- puts "#{plan.contacts.size} contacts in #{plan.group} on #{host}"
+ groups = plan.contacts.flat_map { plan.card(it.id).groups }.uniq
+ puts "#{plan.contacts.size} contacts in #{groups.join(", ")} on #{host}"
end
end
diff --git a/test/pro_tacts/import/test_card.rb b/test/pro_tacts/import/test_card.rb
new file mode 100644
index 0000000..0a201f7
--- /dev/null
+++ b/test/pro_tacts/import/test_card.rb
@@ -0,0 +1,84 @@
+require_relative "../../test_helper"
+
+require "pathname"
+require "tmpdir"
+
+require "pro_tacts/import/card"
+
+class ImportCardTest < Minitest::Test
+ Card = ProTacts::Import::Card
+
+ ADA = <<~YAML
+ first: Ada
+ last: Lovelace
+ phones:
+ - "+12532189075"
+ groups:
+ - sync:*
+ - import-20260916T180412Z
+ YAML
+
+ def read(yaml)
+ Dir.mktmpdir do |tmp|
+ path = Pathname.new(tmp) / "kmnuqmzxylru.yml"
+ path.write(yaml)
+ Card.read(path)
+ end
+ end
+
+ def assert_invalid(yaml, message)
+ error = assert_raises(Card::Invalid) { read(yaml) }
+ assert_includes error.message, message
+ end
+
+ def test_a_card_reads_its_fields
+ assert_equal Card.new(first: "Ada", last: "Lovelace", phones: ["+12532189075"], groups: ["sync:*", "import-20260916T180412Z"]), read(ADA)
+ end
+
+ def test_a_card_is_the_contact_the_web_create_and_add_rows_make
+ contact = read(ADA).contact("kmnuqmzxylru")
+
+ assert_equal "kmnuqmzxylru", contact.id
+ assert_equal "BEGIN:VCARD\r\nVERSION:3.0\r\nN:Lovelace;Ada;;;\r\nFN:Ada Lovelace\r\nUID:kmnuqmzxylru\r\nTEL:+12532189075\r\nEND:VCARD\r\n", contact.vcard.to_s
+ end
+
+ def test_a_name_is_escaped_into_the_card
+ card = Card.new(first: "Ada; Countess", last: "", phones: [], groups: [])
+
+ assert_equal ["Ada; Countess"], [card.contact("kmnuqmzxylru").name]
+ end
+
+ def test_the_document_reads_back_as_the_card
+ card = read(ADA)
+
+ assert_equal card, read(YAML.dump(card.document))
+ end
+
+ def test_a_missing_field_is_refused
+ assert_invalid ADA.sub("phones:\n- \"+12532189075\"\n", ""), "has no phones"
+ end
+
+ def test_a_field_no_card_takes_is_refused
+ assert_invalid "#{ADA}emails: []\n", "has emails, which no field takes"
+ end
+
+ def test_an_unquoted_number_is_refused
+ assert_invalid ADA.sub('"+12532189075"', "+12532189075"), "phones must be a list of quoted numbers"
+ end
+
+ def test_a_name_yaml_reads_as_something_else_is_refused
+ assert_invalid ADA.sub("last: Lovelace", "last: no"), "first and last must be text"
+ end
+
+ def test_a_card_with_no_name_is_refused
+ assert_invalid ADA.sub("first: Ada", "first: ''").sub("last: Lovelace", "last: ' '"), "needs a first or last name"
+ end
+
+ def test_a_blank_group_is_refused
+ assert_invalid ADA.sub("- sync:*", "- ''"), "groups must be a list of names"
+ end
+
+ def test_a_file_that_is_not_a_mapping_is_refused
+ assert_invalid "- Ada\n", "is not a mapping"
+ end
+end
diff --git a/test/pro_tacts/import/test_execute.rb b/test/pro_tacts/import/test_execute.rb
index 3fe135a..8ca99d3 100644
--- a/test/pro_tacts/import/test_execute.rb
+++ b/test/pro_tacts/import/test_execute.rb
@@ -37,25 +37,49 @@ class ImportExecuteTest < Minitest::Test
end
end
- def with_plan(cards = IDS.to_h { [it, card(it, "Contact #{it}")] })
+ # A client whose host refuses every PUT with a precondition body.
+ class RefusingClient < RackClient
+ def call(method, path, headers: {}, body: nil)
+ return Execute::Response.new(status: 412, headers: {}, body: "<error/>") if method == "PUT"
+
+ super
+ end
+ end
+
+ def with_plan(groups: [])
Dir.mktmpdir do |tmp|
dir = Pathname.new(tmp) / "plan"
- entries = cards.map { |id, card| Plan::Entry.new(id:, source_id: "#{id}:ABPerson", card:, backup: {}) }
+ entries = IDS.map { |id|
+ card = ProTacts::Import::Card.new(first: "Contact", last: id, phones: [], groups:)
+ Plan::Entry.new(id:, source_id: "#{id}:ABPerson", card:, backup: {})
+ }
Plan.write(dir, source: "macos", created_at: Time.utc(2026, 9, 16, 18, 4, 12), entries:)
with_contacts({}) { |store| yield Plan.read(dir), store, RackClient.new }
end
end
- def imported_group(store)
- store.all_groups.find { it.name == "import-20260916T180412Z" }
+ def imported_group(store) = group_named(store, "import-20260916T180412Z")
+
+ def everyone(store) = group_named(store, "sync:*")
+
+ def group_named(store, name)
+ store.all_groups.find { it.name == name }
+ end
+
+ def edit_cards(plan)
+ IDS.each do |id|
+ path = plan.dir / "cards/#{id}.yml"
+ path.write(yield(path.read))
+ end
end
def test_every_card_lands_in_the_plans_group
with_plan do |plan, store, client|
Execute.call(plan, host: HOST, client:)
- IDS.each { assert store.contact(it), it }
+ IDS.each { assert_equal "Contact #{it}", store.contact(it).name }
assert_equal IDS.sort, imported_group(store).members.sort
+ assert_equal IDS.sort, everyone(store).members.sort
assert_equal %w[joined joined], Plan.read(plan.dir).contacts.map(&:status)
end
end
@@ -77,7 +101,7 @@ class ImportExecuteTest < Minitest::Test
# A run that died between a write and the plan recording it.
def test_a_card_already_on_the_host_counts_as_landed
with_plan do |plan, store, client|
- client.call("PUT", "/dav/addressbook/#{IDS.first}.vcf", headers: {"Content-Type" => "text/vcard"}, body: plan.card(IDS.first))
+ client.call("PUT", "/dav/addressbook/#{IDS.first}.vcf", headers: {"Content-Type" => "text/vcard"}, body: plan.card(IDS.first).contact(IDS.first).vcard.to_s)
Execute.call(plan, host: HOST, client:)
@@ -106,12 +130,45 @@ class ImportExecuteTest < Minitest::Test
end
def test_a_refused_card_stops_the_run_and_is_not_recorded
- with_plan({"kmnuqmzxylru" => card("someone-else", "Mismatched UID")}) do |plan, store, client|
- error = assert_raises(Execute::Failed) { Execute.call(plan, host: HOST, client:) }
+ with_plan do |plan, _store, _client|
+ error = assert_raises(Execute::Failed) { Execute.call(plan, host: HOST, client: RefusingClient.new) }
- assert_includes error.message, "kmnuqmzxylru"
+ assert_includes error.message, IDS.first
assert_includes error.message, "412"
- assert_equal [nil], Plan.read(plan.dir).contacts.map(&:status)
+ assert_equal [nil, nil], Plan.read(plan.dir).contacts.map(&:status)
+ end
+ end
+
+ def test_a_card_joins_the_groups_its_file_names_making_those_missing
+ with_plan(groups: ["family"]) do |plan, store, client|
+ Execute.call(plan, host: HOST, client:)
+
+ assert_equal IDS.sort, group_named(store, "family").members.sort
+ assert_equal 1, store.all_groups.count { it.name == "family" }
+ end
+ end
+
+ def test_a_card_whose_file_leaves_out_everyone_is_taken_out_of_it
+ with_plan do |plan, store, client|
+ edit_cards(plan) { it.sub("- sync:*\n", "") }
+
+ Execute.call(plan, host: HOST, client:)
+
+ assert_empty everyone(store).members
+ assert_equal IDS.sort, imported_group(store).members.sort
+ end
+ end
+
+ def test_a_card_that_will_not_read_stops_the_run_before_anything_lands
+ with_plan do |plan, store, client|
+ path = plan.dir / "cards/#{IDS.last}.yml"
+ path.write(path.read.sub("last: #{IDS.last}", "last: no"))
+
+ assert_raises(ProTacts::Import::Card::Invalid) { Execute.call(plan, host: HOST, client:) }
+
+ assert_empty client.requests
+ assert_nil Plan.read(plan.dir).host
+ assert_empty store.changes
end
end
end
diff --git a/test/pro_tacts/import/test_macos.rb b/test/pro_tacts/import/test_macos.rb
index a7720d1..31a441f 100644
--- a/test/pro_tacts/import/test_macos.rb
+++ b/test/pro_tacts/import/test_macos.rb
@@ -10,12 +10,16 @@ require "pro_tacts/import/plan"
class ImportMacosTest < Minitest::Test
Macos = ProTacts::Import::Macos
+ Card = ProTacts::Import::Card
+
CREATED_AT = Time.utc(2026, 9, 16, 18, 4, 12)
+ NAME = ["N:Lovelace;Ada;;;", "FN:Ada Lovelace"].freeze
+ GROUPS = ["sync:*", "import-20260916T180412Z"].freeze
- def record(identifier, lines: [], note: nil, image: nil)
+ def record(identifier, name: NAME, lines: [], note: nil, image: nil)
{
"identifier" => identifier,
- "vcard" => ["BEGIN:VCARD", "VERSION:3.0", *lines, "END:VCARD", ""].join("\r\n"),
+ "vcard" => ["BEGIN:VCARD", "VERSION:3.0", *name, *lines, "END:VCARD", ""].join("\r\n"),
"contact" => {"identifier" => identifier, "imageData" => image},
"note" => note
}
@@ -25,7 +29,7 @@ class ImportMacosTest < Minitest::Test
Dir.mktmpdir { yield Pathname.new(it) / "plan" }
end
- def test_an_empty_card_is_planned_under_a_minted_uid
+ def test_a_named_card_is_planned_under_a_minted_id
in_tmpdir do |dir|
plan = Macos.plan(dir, [record("A:ABPerson")], created_at: CREATED_AT)
@@ -33,7 +37,7 @@ class ImportMacosTest < Minitest::Test
contact = plan.contacts.first
assert_match(/\A[k-z]{12}\z/, contact.id)
assert_equal "A:ABPerson", contact.source_id
- assert_equal "BEGIN:VCARD\r\nVERSION:3.0\r\nUID:#{contact.id}\r\nEND:VCARD\r\n", plan.card(contact.id)
+ assert_equal Card.new(first: "Ada", last: "Lovelace", phones: [], groups: GROUPS), plan.card(contact.id)
end
end
@@ -52,19 +56,19 @@ class ImportMacosTest < Minitest::Test
def test_every_unknown_field_is_named_and_no_plan_is_written
in_tmpdir do |dir|
records = [
- record("A:ABPerson", lines: ["N:Lovelace;Ada;;;", "FN:Ada Lovelace"], note: "Analyst."),
- record("B:ABPerson", lines: ["FN:Mary Boole"], image: "/9j/")
+ record("A:ABPerson", lines: ["ORG:Analytical Engines;", "EMAIL:ada@example.com"], note: "Analyst."),
+ record("B:ABPerson", lines: ["EMAIL:mary@example.com"], image: "/9j/")
]
error = assert_raises(Macos::Unknown) { Macos.plan(dir, records, created_at: CREATED_AT) }
assert_equal <<~MESSAGE.chomp, error.message
no branch handles these fields, so no plan was written:
- FN: 2
- A:ABPerson: "FN:Ada Lovelace"
- B:ABPerson: "FN:Mary Boole"
- N: 1
- A:ABPerson: "N:Lovelace;Ada;;;"
+ EMAIL: 2
+ A:ABPerson: "EMAIL:ada@example.com"
+ B:ABPerson: "EMAIL:mary@example.com"
+ ORG: 1
+ A:ABPerson: "ORG:Analytical Engines;"
imageData: 1
B:ABPerson: "/9j/"
note: 1
@@ -94,6 +98,106 @@ class ImportMacosTest < Minitest::Test
end
end
+ def test_a_cell_number_is_a_phone
+ in_tmpdir do |dir|
+ lines = ["TEL;type=CELL;type=VOICE;type=pref:+12532189075", "TEL;type=CELL;type=VOICE;type=pref:+12532189076"]
+ plan = Macos.plan(dir, [record("A:ABPerson", lines:)], created_at: CREATED_AT)
+
+ assert_equal ["+12532189075", "+12532189076"], plan.card(plan.contacts.first.id).phones
+ end
+ end
+
+ def test_a_name_is_read_unescaped
+ in_tmpdir do |dir|
+ plan = Macos.plan(dir, [record("A:ABPerson", name: ["N:O\\,Brien;Ada;;;", "FN:Ada O\\,Brien"])], created_at: CREATED_AT)
+
+ assert_equal "O,Brien", plan.card(plan.contacts.first.id).last
+ end
+ end
+
+ def test_a_name_beyond_first_and_last_is_refused
+ in_tmpdir do |dir|
+ records = [
+ record("A:ABPerson", name: ["N:Lovelace;Ada;Augusta;;", "FN:Ada Lovelace"]),
+ record("B:ABPerson", name: ["N:Lovelace;Ada;;;", "FN:Countess Ada Lovelace"]),
+ record("C:ABPerson", name: ["FN:Ada Lovelace"]),
+ record("D:ABPerson", name: ["N:;;;;", "FN:"])
+ ]
+
+ error = assert_raises(Macos::Unknown) { Macos.plan(dir, records, created_at: CREATED_AT) }
+
+ assert_equal <<~MESSAGE.chomp, error.message
+ no branch handles these fields, so no plan was written:
+ FN other than first and last: 2
+ B:ABPerson: "FN:Countess Ada Lovelace"
+ C:ABPerson: "FN:Ada Lovelace"
+ N beyond first and last: 1
+ A:ABPerson: "N:Lovelace;Ada;Augusta;;"
+ N lines: 0: 1
+ C:ABPerson: ""
+ no name: 1
+ D:ABPerson: "N:;;;;"
+ MESSAGE
+ end
+ end
+
+ def test_an_apple_prodid_is_dropped
+ in_tmpdir do |dir|
+ plan = Macos.plan(dir, [record("A:ABPerson", lines: ["PRODID:-//Apple Inc.//macOS 26.0//EN"])], created_at: CREATED_AT)
+
+ assert_equal Card.new(first: "Ada", last: "Lovelace", phones: [], groups: GROUPS), plan.card(plan.contacts.first.id)
+ end
+ end
+
+ def test_any_other_prodid_is_refused
+ in_tmpdir do |dir|
+ records = [
+ record("A:ABPerson", lines: ["PRODID:-//Monica//EN"]),
+ record("B:ABPerson", lines: ["PRODID;VALUE=text:-//Apple Inc.//macOS 26.0//EN"])
+ ]
+
+ error = assert_raises(Macos::Unknown) { Macos.plan(dir, records, created_at: CREATED_AT) }
+
+ assert_equal <<~MESSAGE.chomp, error.message
+ no branch handles these fields, so no plan was written:
+ PRODID value: 1
+ A:ABPerson: "PRODID:-//Monica//EN"
+ PRODID;VALUE=text: 1
+ B:ABPerson: "PRODID;VALUE=text:-//Apple Inc.//macOS 26.0//EN"
+ MESSAGE
+ end
+ end
+
+ def test_a_line_in_any_other_form_is_refused_under_that_form
+ in_tmpdir do |dir|
+ records = [
+ record("A:ABPerson", lines: ["TEL;type=HOME;type=VOICE:+12532189075"]),
+ record("B:ABPerson", lines: ["TEL;type=CELL;type=VOICE:+12532189075"]),
+ record("C:ABPerson", lines: ["item1.TEL;type=CELL;type=VOICE;type=pref:+12532189075"]),
+ record("D:ABPerson", lines: ["TEL;type=CELL;type=VOICE;type=pref:(253) 218-9075"]),
+ record("E:ABPerson", name: ["N:Lovelace;Ada;;;", "FN;CHARSET=utf-8:Ada Lovelace"])
+ ]
+
+ error = assert_raises(Macos::Unknown) { Macos.plan(dir, records, created_at: CREATED_AT) }
+
+ assert_equal <<~MESSAGE.chomp, error.message
+ no branch handles these fields, so no plan was written:
+ FN lines: 0: 1
+ E:ABPerson: ""
+ FN;CHARSET=utf-8: 1
+ E:ABPerson: "FN;CHARSET=utf-8:Ada Lovelace"
+ TEL value: 1
+ D:ABPerson: "TEL;type=CELL;type=VOICE;type=pref:(253) 218-9075"
+ TEL;type=CELL;type=VOICE: 1
+ B:ABPerson: "TEL;type=CELL;type=VOICE:+12532189075"
+ TEL;type=HOME;type=VOICE: 1
+ A:ABPerson: "TEL;type=HOME;type=VOICE:+12532189075"
+ item1.TEL;type=CELL;type=VOICE;type=pref: 1
+ C:ABPerson: "item1.TEL;type=CELL;type=VOICE;type=pref:+12532189075"
+ MESSAGE
+ end
+ end
+
def test_a_card_that_is_not_version_3_is_refused
in_tmpdir do |dir|
source = record("A:ABPerson")
diff --git a/test/pro_tacts/import/test_plan.rb b/test/pro_tacts/import/test_plan.rb
index fe493f3..7a68ed4 100644
--- a/test/pro_tacts/import/test_plan.rb
+++ b/test/pro_tacts/import/test_plan.rb
@@ -1,6 +1,5 @@
require_relative "../../test_helper"
-require "json"
require "pathname"
require "tmpdir"
@@ -13,8 +12,10 @@ class ImportPlanTest < Minitest::Test
CREATED_AT = Time.utc(2026, 9, 16, 18, 4, 12)
- def entry(id, source_id: "#{id}:ABPerson", backup: {"original.vcf" => "BEGIN:VCARD\r\n"})
- Plan::Entry.new(id:, source_id:, card: card(id, "Ada Lovelace"), backup:)
+ ADA = ProTacts::Import::Card.new(first: "Ada", last: "Lovelace", phones: ["+12532189075"], groups: [])
+
+ def entry(id, source_id: "#{id}:ABPerson", card: ADA, backup: {"original.vcf" => "BEGIN:VCARD\r\n"})
+ Plan::Entry.new(id:, source_id:, card:, backup:)
end
def in_tmpdir
@@ -30,9 +31,7 @@ class ImportPlanTest < Minitest::Test
assert_equal "macos", plan.source
assert_equal "2026-09-16T18:04:12Z", plan.created_at
assert_equal [Plan::Contact.new(id: "kmnuqmzxylru", source_id: "kmnuqmzxylru:ABPerson", status: nil), Plan::Contact.new(id: "vmnlryyvktux", source_id: "vmnlryyvktux:ABPerson", status: nil)], plan.contacts
- assert_equal card("kmnuqmzxylru", "Ada Lovelace"), plan.card("kmnuqmzxylru")
assert_nil plan.host
- assert_nil plan.group_id
end
end
@@ -44,6 +43,35 @@ class ImportPlanTest < Minitest::Test
end
end
+ def test_a_card_joins_everyone_and_the_plans_group_after_its_own
+ in_tmpdir do |dir|
+ card = ProTacts::Import::Card.new(first: "Ada", last: "Lovelace", phones: ["+12532189075"], groups: ["family"])
+ Plan.write(dir, source: "macos", created_at: CREATED_AT, entries: [entry("kmnuqmzxylru", card:)])
+
+ assert_equal <<~YAML, (dir / "cards/kmnuqmzxylru.yml").read
+ ---
+ first: Ada
+ last: Lovelace
+ phones:
+ - "+12532189075"
+ groups:
+ - sync:*
+ - import-20260916T180412Z
+ - family
+ YAML
+ end
+ end
+
+ def test_a_card_reads_back_with_its_edits
+ in_tmpdir do |dir|
+ plan = Plan.write(dir, source: "macos", created_at: CREATED_AT, entries: [entry("kmnuqmzxylru")])
+ path = dir / "cards/kmnuqmzxylru.yml"
+ path.write(path.read.sub("first: Ada", "first: Augusta Ada"))
+
+ assert_equal "Augusta Ada", plan.card("kmnuqmzxylru").first
+ end
+ end
+
def test_the_backup_is_written_as_given
in_tmpdir do |dir|
photo = "\xFF\xD8\xFF".b
@@ -69,13 +97,11 @@ class ImportPlanTest < Minitest::Test
plan.host = "https://contacts"
plan.record("kmnuqmzxylru", "landed")
- plan.group_id = "kxsv"
read = Plan.read(dir)
assert_equal "https://contacts", read.host
- assert_equal "kxsv", read.group_id
assert_equal ["landed", nil], read.contacts.map(&:status)
- assert_equal ["plan.json"], dir.children.map { it.basename.to_s }.grep(/plan/)
+ assert_equal ["plan.yml"], dir.children.map { it.basename.to_s }.grep(/plan/)
end
end
end