Replace the Contacts store with a Contact data class
Contact.all parses the directory eagerly and lookups happen over the
parsed contacts, so client-supplied ids never reach the filesystem.
Assisted-by: GLM-5.3 via pi
diff --git a/lib/pro_tacts/contact.rb b/lib/pro_tacts/contact.rb
new file mode 100644
index 0000000..c8882e3
--- /dev/null
+++ b/lib/pro_tacts/contact.rb
@@ -0,0 +1,59 @@
+require "kdl"
+require "pathname"
+require "sentry-ruby"
+
+require "pro_tacts"
+require "pro_tacts/vcard"
+
+module ProTacts
+ # A contact parsed from one KDL file under the contacts directory; the
+ # filename is the id, which maps to the vCard UID (see
+ # docs/plans/2026-01-12-carddav-architecture.md).
+ class Contact < Data.define(:id, :vcard)
+ # Ids end up in paths and arrive from client-supplied hrefs, so a
+ # filename outside this charset is skipped at load; everything
+ # listed is then fetchable by its id.
+ ID_FORMAT = /\A[\w-]+\z/
+
+ # Every contact in the directory, one file per contact. An empty
+ # directory is a valid empty address book, but any non-hidden
+ # non-.kdl file raises — a misplaced file or a wrong
+ # PRO_TACTS_CONTACTS_DIR should not quietly serve a partial address
+ # book. Files that fail to parse or render are reported and skipped;
+ # Sentry is a no-op while uninitialized, so tests need no DSN.
+ def self.all(directory = ProTacts.config.contacts_dir)
+ directory = Pathname.new(directory)
+ raise ArgumentError, "contacts directory not found: #{directory}" unless directory.directory?
+
+ unexpected = directory.children
+ .reject { it.basename.to_s.start_with?(".") }
+ .reject { it.extname == ".kdl" }
+ unless unexpected.empty?
+ raise ArgumentError, "unexpected non-KDL file in contacts directory: #{unexpected.first}"
+ end
+
+ directory.glob("*.kdl").filter_map do |path|
+ parse(path)
+ rescue KDL::Error, ArgumentError, SystemCallError => e
+ Sentry.capture_message("skipping contact file #{path}: #{e.class}: #{e.message}")
+ end
+ end
+
+ # File to Contact; raises on anything that would make the contact
+ # unloadable. `all` decides what to do about it.
+ def self.parse(path)
+ path = Pathname.new(path)
+ id = path.basename(".kdl").to_s
+ unless id.match?(ID_FORMAT)
+ raise ArgumentError, "invalid contact id: #{id}"
+ end
+
+ nodes = KDL.parse(path.read).nodes
+ unless nodes.length == 1 && nodes.first.name == "contact"
+ raise ArgumentError, "expected exactly one contact node"
+ end
+
+ new(id:, vcard: VCard.render(nodes.first, uid: id))
+ end
+ end
+end
diff --git a/lib/pro_tacts/contacts.rb b/lib/pro_tacts/contacts.rb
deleted file mode 100644
index 1ac28d7..0000000
--- a/lib/pro_tacts/contacts.rb
+++ /dev/null
@@ -1,74 +0,0 @@
-require "kdl"
-require "pathname"
-require "sentry-ruby"
-
-require "pro_tacts/vcard"
-
-module ProTacts
- # Reads contacts from a directory of KDL files, one contact per file.
- # The filename minus extension is the contact ID, which maps to the
- # vCard UID (see docs/plans/2026-01-12-carddav-architecture.md).
- class Contacts
- Contact = Data.define(:id, :vcard)
-
- # IDs end up in paths, and they arrive from client-supplied hrefs, so
- # anything outside this charset simply does not exist. Enforced at
- # load too, so everything listed is fetchable by that id.
- ID_FORMAT = /\A[\w-]+\z/
-
- attr_reader :directory
-
- def initialize(directory)
- @directory = Pathname.new(directory)
- unless @directory.directory?
- raise ArgumentError, "contacts directory not found: #{@directory}"
- end
- end
-
- # An empty directory is a valid empty address book. Anything in it
- # that is not a .kdl file means a misplaced file or a wrong
- # PRO_TACTS_CONTACTS_DIR, so it raises rather than quietly serving a
- # partial address book. Dotfiles are exempt: Finder drops .DS_Store
- # into any directory it opens.
- def all
- unexpected = directory.children
- .reject { it.basename.to_s.start_with?(".") }
- .reject { it.extname == ".kdl" }
- unless unexpected.empty?
- raise ArgumentError, "unexpected non-KDL file in contacts directory: #{unexpected.first}"
- end
-
- directory.glob("*.kdl").map { load(it) }.compact
- end
-
- def find(id)
- return nil unless id.match?(ID_FORMAT)
-
- path = directory / "#{id}.kdl"
- load(path) if path.file?
- end
-
- private
-
- # Rendering at load time doubles as validation: a file whose card
- # cannot render is reported and skipped rather than taking the whole
- # address book down with it. Sentry is a no-op while uninitialized,
- # so tests need no DSN.
- def load(path)
- id = path.basename(".kdl").to_s
- unless id.match?(ID_FORMAT)
- raise ArgumentError, "invalid contact id: #{id}"
- end
-
- nodes = KDL.parse(path.read).nodes
- unless nodes.length == 1 && nodes.first.name == "contact"
- raise ArgumentError, "expected exactly one contact node"
- end
-
- Contact.new(id:, vcard: VCard.render(nodes.first, uid: id))
- rescue KDL::Error, ArgumentError, SystemCallError => e
- Sentry.capture_message("skipping contact file #{path}: #{e.class}: #{e.message}")
- nil
- end
- end
-end
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index b237454..b4447f8 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -25,7 +25,7 @@ require "nokogiri"
require "roda"
require "pro_tacts/debug_logger"
-require "pro_tacts/contacts"
+require "pro_tacts/contact"
require "roda/plugins/dav_verbs"
module ProTacts
@@ -180,7 +180,7 @@ module ProTacts
end
# Depth: 0 returns only collection, Depth: 1 includes members
- members = depth == "0" ? "" : contacts.all.map { etag_response(it.id) }.join
+ members = depth == "0" ? "" : contacts.map { etag_response(it.id) }.join
<<~XML
<?xml version="1.0" encoding="UTF-8"?>
@@ -204,13 +204,13 @@ module ProTacts
if doc.root.name == "sync-collection"
# The warm-sync ask is etag-only; a changed etag sends the
# client back through multiget, so no address-data here.
- responses = contacts.all.map { etag_response(it.id) }
+ responses = contacts.map { etag_response(it.id) }
else
wants_cards = doc.xpath("//address-data").any?
responses = doc.xpath("//href").map { it.text }.map { |requested|
id = requested[%r{\A/dav/addressbook/([^/]+)\.vcf\z}, 1]
- contact = id && contacts.find(id)
+ contact = id && contacts.find { it.id == id }
if contact
wants_cards ? card_response(contact) : etag_response(contact.id)
@@ -229,7 +229,7 @@ module ProTacts
end
r.get String do |filename|
- contact = contacts.find(filename.delete_suffix(".vcf"))
+ contact = contacts.find { it.id == filename.delete_suffix(".vcf") }
# No match falls through to the empty-body 404 that the
# not_found handler fills in.
@@ -245,10 +245,11 @@ module ProTacts
private
- # Instantiated per request so tests can point it at a fixture
- # directory through ProTacts.config=; caching belongs with real etags.
+ # Parsed once per request — Roda builds a fresh app instance for each
+ # one — with the directory coming from config. Caching belongs with
+ # real etags.
def contacts
- @contacts ||= Contacts.new(ProTacts.config.contacts_dir)
+ @contacts ||= Contact.all
end
def contact_href(id)
diff --git a/test/pro_tacts/test_contacts.rb b/test/pro_tacts/test_contact.rb
similarity index 53%
rename from test/pro_tacts/test_contacts.rb
rename to test/pro_tacts/test_contact.rb
index d4f68f7..c8392fd 100644
--- a/test/pro_tacts/test_contacts.rb
+++ b/test/pro_tacts/test_contact.rb
@@ -2,78 +2,53 @@ require_relative "../test_helper"
require "tmpdir"
-require "pro_tacts/contacts"
+require "pro_tacts/contact"
-class ContactsTest < Minitest::Test
+class ContactTest < Minitest::Test
def with_contacts(files)
Dir.mktmpdir do |dir|
- directory = Pathname.new(dir)
- files.each { |name, content| (directory / name).write(content) }
- yield ProTacts::Contacts.new(directory)
+ files.each { |name, content| File.write(File.join(dir, name), content) }
+ yield ProTacts::Contact.all(dir)
end
end
- def test_all_lists_every_contact
+ def test_all_parses_every_contact
with_contacts({
"znorth.kdl" => "contact { name \"Zed\" }",
"aiden.kdl" => "contact { name \"Aiden\" }",
}) do |contacts|
- ids = contacts.all.map { it.id }
+ ids = contacts.map { it.id }
assert_includes ids, "aiden"
assert_includes ids, "znorth"
end
end
- def test_find_returns_the_contact_by_id
- with_contacts({"aiden.kdl" => "contact { name \"Aiden\" }"}) do |contacts|
- contact = contacts.find("aiden")
-
- assert_equal "aiden", contact.id
- assert_includes contact.vcard, "FN:Aiden"
- end
- end
-
def test_the_uid_comes_from_the_filename
with_contacts({"kqmtnwpxlrvszoyp.kdl" => "contact { name \"Aiden\" }"}) do |contacts|
- assert_includes contacts.find("kqmtnwpxlrvszoyp").vcard, "UID:kqmtnwpxlrvszoyp"
+ assert_includes contacts.first.vcard, "UID:kqmtnwpxlrvszoyp"
end
end
- def test_find_returns_nil_for_unknown_ids
- with_contacts({"aiden.kdl" => "contact { name \"Aiden\" }"}) do |contacts|
- assert_nil contacts.find("nope")
- end
- end
-
- def test_find_rejects_ids_outside_the_charset
+ def test_an_empty_directory_lists_no_contacts
with_contacts({}) do |contacts|
- assert_nil contacts.find("../secrets")
- assert_nil contacts.find("a/b")
- assert_nil contacts.find("a.vcf")
+ assert_empty contacts
end
end
def test_a_missing_directory_raises
error = assert_raises(ArgumentError) do
- ProTacts::Contacts.new(Pathname.new(Dir.mktmpdir) / "nonexistent")
+ ProTacts::Contact.all(Pathname.new(Dir.mktmpdir) / "nonexistent")
end
assert_match(/contacts directory not found/, error.message)
end
- def test_an_empty_directory_lists_no_contacts
- with_contacts({}) do |contacts|
- assert_empty contacts.all
- end
- end
-
def test_non_kdl_files_raise
- with_contacts({
- "aiden.kdl" => "contact { name \"Aiden\" }",
- "notes.txt" => "hello",
- }) do |contacts|
- error = assert_raises(ArgumentError) { contacts.all }
+ Dir.mktmpdir do |dir|
+ File.write(File.join(dir, "notes.txt"), "hello")
+
+ error = assert_raises(ArgumentError) { ProTacts::Contact.all(dir) }
assert_match(/non-KDL file/, error.message)
assert_match(/notes\.txt/, error.message)
@@ -85,7 +60,7 @@ class ContactsTest < Minitest::Test
".DS_Store" => "junk",
"aiden.kdl" => "contact { name \"Aiden\" }",
}) do |contacts|
- assert_equal %w[aiden], contacts.all.map { it.id }
+ assert_equal %w[aiden], contacts.map { it.id }
end
end
@@ -94,7 +69,7 @@ class ContactsTest < Minitest::Test
"John Smith.kdl" => "contact { name \"John\" }",
"aiden.kdl" => "contact { name \"Aiden\" }",
}) do |contacts|
- assert_equal %w[aiden], contacts.all.map { it.id }
+ assert_equal %w[aiden], contacts.map { it.id }
end
end
@@ -103,8 +78,7 @@ class ContactsTest < Minitest::Test
"broken.kdl" => "contact {",
"aiden.kdl" => "contact { name \"Aiden\" }",
}) do |contacts|
- assert_equal %w[aiden], contacts.all.map { it.id }
- assert_nil contacts.find("broken")
+ assert_equal %w[aiden], contacts.map { it.id }
end
end
@@ -114,7 +88,7 @@ class ContactsTest < Minitest::Test
"two.kdl" => "contact { name \"A\" }\ncontact { name \"B\" }",
"other.kdl" => "person { name \"A\" }",
}) do |contacts|
- assert_empty contacts.all
+ assert_empty contacts
end
end
@@ -123,8 +97,7 @@ class ContactsTest < Minitest::Test
"nameless.kdl" => "contact { phone \"+1-555-1234\" }",
"aiden.kdl" => "contact { name \"Aiden\" }",
}) do |contacts|
- assert_equal %w[aiden], contacts.all.map { it.id }
- assert_nil contacts.find("nameless")
+ assert_equal %w[aiden], contacts.map { it.id }
end
end
end