Re-id contacts the server did not mint
Assisted-by: GLM-5.3 via pi
diff --git a/AGENTS.md b/AGENTS.md
index c7a4b39..e67116c 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -17,6 +17,8 @@ rake fixtures # Re-record response fixtures from current behavior
rake fixtures:extract # A logged exchange's request as a fixture step
# (EXCHANGE=id STEP=test/fixtures/<recording>/NN-name)
rake index:rebuild # Derive the parsed index again from the stored cards
+rake contacts:reid # Move contacts whose ids are not server-minted
+ # onto minted ones, rewriting each card's UID
rake book:name # Name a login's book and rename its sync group
# (LOGIN=login NAME=name; no NAME clears it)
rake db:dump # Write the cards, birthdays, and groups to data/dump
diff --git a/lib/pro_tacts/change_id.rb b/lib/pro_tacts/change_id.rb
index 7e07e6d..1c1bd3c 100644
--- a/lib/pro_tacts/change_id.rb
+++ b/lib/pro_tacts/change_id.rb
@@ -8,11 +8,26 @@ module ProTacts
module ChangeId
ALPHABET = "klmnopqrstuvwxyz".chars.freeze #: Array[String]
+ # The length a contact's id is minted at. Store#put overwrites
+ # rather than refuses, so the width is the only guard a create has:
+ # 48 bits, for a family's book. The admin create draws it (Web's
+ # contacts branch); Store#reid measures ids against it.
+ CONTACT_LENGTH = 12 #: Integer
+
# SecureRandom rather than rand: the draws have to be independent of
# anything a caller can observe or seed.
#: (Integer length) -> String
def self.mint(length)
SecureRandom.alphanumeric(length, chars: ALPHABET)
end
+
+ # Whether an id is one this server minted for a contact: the shape
+ # #mint draws at CONTACT_LENGTH, and nothing else. Ids that fail
+ # this — a client's UUID, a seed from before the shape settled —
+ # are what rake contacts:reid moves.
+ #: (String id) -> bool
+ def self.minted?(id)
+ /\A[#{ALPHABET.join}]{#{CONTACT_LENGTH}}\z/.match?(id)
+ end
end
end
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index b38989f..f17220d 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -149,6 +149,12 @@ module ProTacts
# of bad draws — better to say so than to spin.
GROUP_ID_ATTEMPTS = 8 #: Integer
+ # How many ids a re-id draws before giving up. Twelve letters are
+ # one of 2^48, so GROUP_ID_ATTEMPTS' reasoning arrives intact at a
+ # wider alphabet: a first collision is a broken generator rather
+ # than a run of bad draws.
+ CONTACT_ID_ATTEMPTS = 8 #: Integer
+
# The property names whose value is structured rather than free
# text (RFC 2426 section 3.2.1), among the two a group may lend:
# ADR is components, NOTE is text (db/migrations/004_groups.rb).
@@ -576,6 +582,69 @@ module ProTacts
end
end
+ # Gives a contact the id this server would have minted for it, and
+ # hands the new id back. Every row that names the card moves with
+ # it — the birthday, the memberships, the index, and the change
+ # log's history, because the contact is the same contact and its
+ # page keeps its record. The stored card's UID is rewritten to
+ # match: a PUT must carry the id as its UID (Web#write_card), so a
+ # card still spelling the old id could never be written again. A
+ # card with no UID, or several, comes out with the one line — the
+ # shape every write through #put already leaves.
+ #
+ # What a syncing client hears is written new rather than moved:
+ # the old href's delete beside the new one's put, one transaction,
+ # because a token silently skips whatever the log missed (see
+ # #put). Without the pair, a client holding an older token would
+ # keep serving the dead href out of its cache forever.
+ #: (String id) -> String
+ def reid(id)
+ # `sole` rather than #contact's nil-for-404: a read whose filter
+ # means one row, and no row here is the caller's mistake to hear
+ # about as Sequel::NoMatchingRow, not a request to answer.
+ before = contact_from(cards.where(id:).sole, birthday_of(id), inherited_of(id))
+
+ CONTACT_ID_ATTEMPTS.times do
+ new_id = ChangeId.mint(ChangeId::CONTACT_LENGTH)
+ # A card with no UID line gains one here, and several collapse
+ # to it: #insert lands it before END:VCARD, the envelope's own
+ # last word, where every card this server writes leaves it.
+ _, rest = before.stored.extract("UID")
+ stored = rest.insert(["UID:#{new_id}\r\n"])
+ begin
+ @database.transaction do
+ # Deferred rather than ordered: every child row names the id
+ # the cards row is leaving, so no sequence of plain updates
+ # satisfies a foreign key that checks per row — the children
+ # cannot move before the parent does, and the parent cannot
+ # move while the children remain. `defer_foreign_keys` is
+ # the one FK pragma that works inside a transaction (it
+ # resets at commit); `foreign_keys` itself is a no-op there
+ # (db/migrations/005_group_identity.rb records the reason).
+ @database.run("PRAGMA defer_foreign_keys = ON")
+ cards.where(id:).update(id: new_id, vcard: stored.to_s, updated_at: NOW)
+ birthdays.where(card_id: id).update(card_id: new_id)
+ group_members.where(card_id: id).update(card_id: new_id)
+ change_log.where(card_id: id).update(card_id: new_id)
+ # The index is a projection, dropped and re-derived below
+ # rather than moved — the deal #reindex always gives it. The
+ # parameters follow their properties away on the cascade.
+ card_properties.where(card_id: id).delete
+ after = contact_from(cards.where(id: new_id).sole, birthday_of(new_id), inherited_of(new_id))
+ record(id, "delete", nil, CardDiff.between(before.vcard, nil))
+ record(new_id, "put", after.etag, CardDiff.between(before.vcard, after.vcard))
+ reindex(new_id, stored)
+ end
+ return new_id
+ rescue Sequel::UniqueConstraintViolation
+ # The minted id collided with a card already holding it; the
+ # only unique constraint this write can hit.
+ next
+ end
+ end
+ raise "no free contact id in #{CONTACT_ID_ATTEMPTS} draws"
+ end
+
# Creates a group and hands back the id it was given. The name is
# the author's label and optional; a group without one is displayed
# by its id (see #inherited_rows), and the empty string is refused
diff --git a/lib/pro_tacts/web/contacts.rb b/lib/pro_tacts/web/contacts.rb
index 5bda5db..c3e1a13 100644
--- a/lib/pro_tacts/web/contacts.rb
+++ b/lib/pro_tacts/web/contacts.rb
@@ -49,9 +49,7 @@ module ProTacts
if first.empty? && last.empty?
dashboard(query: r.params["q"], notice: "A contact needs a name.")
else
- # Store#put overwrites, so the width has to make a collision
- # unthinkable rather than caught: 48 bits, for a family's book.
- id = ChangeId.mint(12)
+ id = ChangeId.mint(ChangeId::CONTACT_LENGTH)
store.put(id, Admin::CardForm.new_card(id, first, middle, last))
r.redirect "/contacts/#{id}", 303
end
diff --git a/tasks/contacts.rake b/tasks/contacts.rake
new file mode 100644
index 0000000..33da898
--- /dev/null
+++ b/tasks/contacts.rake
@@ -0,0 +1,24 @@
+# Contacts whose ids this server did not mint: a client's creates
+# arrive as UUIDs (macOS mints one into both the URI and the card's
+# UID), and the seeds predate the shape. Store#reid moves each onto an
+# id of the server's own — a change every syncing client hears through
+# the change log's delete-and-put pair, hrefs and all.
+
+namespace :contacts do
+ desc "Re-id contacts whose ids are not server-minted (client UUIDs and old seeds)"
+ task :reid do
+ require "pro_tacts"
+ require "pro_tacts/store"
+
+ ProTacts::Store.connect(ProTacts.config.database_path) do |store|
+ unminted = store.contacts.reject { ProTacts::ChangeId.minted?(it.id) }
+ if unminted.empty?
+ puts "every contact's id is server-minted"
+ else
+ unminted.each do |contact|
+ puts "#{contact.id} -> #{store.reid(contact.id)}"
+ end
+ end
+ end
+ end
+end
diff --git a/test/pro_tacts/test_change_id.rb b/test/pro_tacts/test_change_id.rb
index 07f443c..b78feda 100644
--- a/test/pro_tacts/test_change_id.rb
+++ b/test/pro_tacts/test_change_id.rb
@@ -12,4 +12,20 @@ class ChangeIdTest < Minitest::Test
def test_the_draws_cover_the_whole_alphabet
assert_equal ProTacts::ChangeId::ALPHABET.sort, ProTacts::ChangeId.mint(1_000).chars.uniq.sort
end
+
+ def test_a_minted_contact_id_is_twelve_letters_from_the_alphabet
+ assert ProTacts::ChangeId.minted?(ProTacts::ChangeId.mint(ProTacts::ChangeId::CONTACT_LENGTH))
+ end
+
+ def test_a_client_uuid_is_not_a_minted_contact_id
+ refute ProTacts::ChangeId.minted?("AB12C345-6789-0DEF-1234-567890ABCDEF")
+ end
+
+ def test_a_group_id_is_not_a_minted_contact_id
+ refute ProTacts::ChangeId.minted?(ProTacts::ChangeId.mint(4))
+ end
+
+ def test_letters_outside_the_alphabet_are_not_minted
+ refute ProTacts::ChangeId.minted?("abcdefghijjj")
+ end
end
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index bb50c1c..ebd5a71 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -20,6 +20,9 @@ class StoreTest < Minitest::Test
# UTC ISO 8601 to the millisecond, which is what SQLite is asked for.
TIMESTAMP = /\A\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d{3}Z\z/
ZED = "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Zed\r\nUID:znorth\r\nEND:VCARD\r\n"
+ # The id a macOS create mints into both the URI and the card's UID.
+ UUID = "AB12C345-6789-0DEF-1234-567890ABCDEF" #: String
+ UUID_CARD = "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Aiden\r\nUID:#{UUID}\r\nEND:VCARD\r\n" #: String
# A store on disk rather than in memory: WAL, the busy timeout, and
# reopening a database are all part of what is under test.
@@ -445,6 +448,65 @@ class StoreTest < Minitest::Test
end
end
+ ## Re-id
+
+ def test_a_reid_moves_the_contact_onto_a_minted_id
+ with_store({UUID => UUID_CARD}) do |store|
+ new_id = store.reid(UUID)
+
+ assert ProTacts::ChangeId.minted?(new_id)
+ assert_nil store.contact(UUID)
+ assert_equal "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Aiden\r\nUID:#{new_id}\r\nEND:VCARD\r\n",
+ store.contact(new_id).vcard.to_s
+ end
+ end
+
+ def test_a_reid_moves_the_birthday_the_memberships_and_the_index
+ with_store({UUID => "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Aiden\r\nUID:#{UUID}\r\nBDAY:1985-04-12\r\nEND:VCARD\r\n"}) do |store|
+ group = store.create_group(name: "Booles")
+ store.add_member(group, UUID)
+
+ new_id = store.reid(UUID)
+
+ assert_equal ProTacts::Birthday.new(year: 1985, month: 4, day: 12), store.contact(new_id).birthday
+ assert_equal [new_id], store.group(group).members
+ assert_equal new_id, store.card_id_with_uid(new_id)
+ assert database(store)[:card_properties].where(card_id: UUID).empty?
+ end
+ end
+
+ def test_a_reid_tells_the_log_what_a_syncing_client_must_hear
+ with_store({UUID => UUID_CARD}) do |store|
+ new_id = store.reid(UUID)
+
+ # The original put now names the new id, and the delete beside
+ # the new put is the pair that moves a client off the old href.
+ assert_equal [[new_id, "put"], [UUID, "delete"], [new_id, "put"]],
+ store.changes.map { [it.card_id, it.action] }
+ # An old token's delta: both hrefs, the old one to hear it is gone.
+ assert_equal [new_id, UUID], store.changes(after: 0).map { it.card_id }.uniq
+
+ assert_equal store.contact(new_id).etag, store.changes.last.etag
+ delete = store.changes.fetch(1)
+ assert_equal UUID_CARD.lines.map(&:chomp), delete.diff.removed
+ assert_equal %w[put put], store.changes_of(new_id).map { it.action }
+ end
+ end
+
+ def test_a_reid_gives_a_card_with_no_uid_line_one
+ with_store({"aiden" => "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Aiden\r\nEND:VCARD\r\n"}) do |store|
+ new_id = store.reid("aiden")
+
+ assert_equal "UID:#{new_id}", store.contact(new_id).vcard.to_s.lines.fetch(-2).chomp
+ end
+ end
+
+ def test_reiding_an_unknown_contact_raises
+ with_store({}) do |store|
+ assert_raises(Sequel::NoMatchingRow) { store.reid("nope") }
+ end
+ end
+
def test_a_first_write_adds_the_whole_card
with_store({"aiden" => AIDEN}) do |store|
diff = store.changes.last.diff
diff --git a/test/tasks/test_contacts.rb b/test/tasks/test_contacts.rb
new file mode 100644
index 0000000..a3d6cb3
--- /dev/null
+++ b/test/tasks/test_contacts.rb
@@ -0,0 +1,56 @@
+require_relative "../test_helper"
+
+require "pathname"
+require "rake"
+require "tmpdir"
+
+require "pro_tacts/change_id"
+require "pro_tacts/store"
+require "pro_tacts/vcard"
+
+class ContactsTasksTest < Minitest::Test
+ # The id a macOS create mints into both the URI and the card's UID,
+ # and the one the server itself would have minted.
+ UUID = "AB12C345-6789-0DEF-1234-567890ABCDEF" #: String
+ MINTED = "zzzzzzzzzzzz" #: String
+
+ def test_reid_moves_the_contacts_the_server_did_not_mint
+ Dir.mktmpdir do |root|
+ root = Pathname.new(root)
+ ProTacts::Store.connect(root / "contacts.db") do |store|
+ store.put(UUID, vcard("BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Aiden\r\nUID:#{UUID}\r\nEND:VCARD\r\n"))
+ store.put(MINTED, vcard("BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Zed\r\nUID:#{MINTED}\r\nEND:VCARD\r\n"))
+
+ out, = run_task(root)
+
+ assert_match(/\A#{UUID} -> [k-z]{12}\n\z/, out)
+ ids = store.contacts.map { it.id }
+ assert_includes ids, MINTED
+ assert ids.all? { ProTacts::ChangeId.minted?(it) }
+
+ out, = run_task(root)
+ assert_equal "every contact's id is server-minted\n", out
+ end
+ end
+ end
+
+ private
+
+ # A card from bytes, the shape Store#put takes.
+ def vcard(bytes) = ProTacts::VCard.new(bytes)
+
+ # Runs contacts:reid in a Rake application of its own, with root as
+ # the data directory.
+ def run_task(root)
+ config = ProTacts.config
+ application = Rake.application
+
+ ProTacts.config = ProTacts::Config.new("PRO_TACTS_DATA_DIR" => root.to_s)
+ Rake.application = Rake::Application.new
+ load (Pathname.new(__dir__).parent.parent / "tasks" / "contacts.rake").to_s
+ capture_io { Rake.application["contacts:reid"].invoke }
+ ensure
+ ProTacts.config = config
+ Rake.application = application
+ end
+end