vcard: a card knows which assumptions it broke
Store was reading Parser's error taxonomy directly to tell a broken
assumption from ordinary bad input. The reporting stays in Store: a
parse runs on every read — the admin index parses every contact on
every page load — so a parser that reported would alert once per page
view rather than once per arrival.
Assisted-by: Claude Opus 5 via Claude Code
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 8872724..b9ebd11 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -428,7 +428,7 @@ module ProTacts
# A line that broke a parser assumption is not an unrecognized
# BDAY, it is a line this server declined to read at all —
# report_broken_assumptions says so, and more precisely.
- next false if line.error.is_a?(VCard::Parser::BrokenAssumption)
+ next false if line.broke_assumption?
property = bday_of(line)
property.nil? || (!Birthday.rendered?(property) && !Birthday.unrendered_value?(property.value))
@@ -455,16 +455,22 @@ module ProTacts
end
# The assumption report, louder than the two above it: a submitted
- # card that breaks one of the parser's assumptions
- # (VCard::Parser::BrokenAssumption) is not bad input, it is news
- # that the assumption is wrong and every simplification resting on
- # it is now suspect. Nothing else would say so — the card is stored
- # and served either way, and only its index entry is lost. The
- # message carries no card content (ProTacts::SentryScrubber's
- # line); the admin view shows the card raw.
+ # card that breaks one of the parser's assumptions is not bad
+ # input, it is news that the assumption is wrong and every
+ # simplification resting on it is now suspect. Nothing else would
+ # say so — the card is stored and served either way, and only its
+ # index entry is lost.
+ #
+ # Here rather than in the parser, which is where the break is
+ # found: a parse happens on every read of a card — the admin index
+ # parses every contact on every page load — and one bad card must
+ # not alert once per page view. An arrival is the event worth a
+ # message. The message carries no card content
+ # (ProTacts::SentryScrubber's line); the admin view shows the card
+ # raw.
#: (VCard card) -> void
def report_broken_assumptions(card)
- broken = card.lines.count { it.error.is_a?(VCard::Parser::BrokenAssumption) }
+ broken = card.broken_assumptions.length
return if broken.zero?
Sentry.capture_message(
diff --git a/lib/pro_tacts/vcard.rb b/lib/pro_tacts/vcard.rb
index 58fb859..d39817c 100644
--- a/lib/pro_tacts/vcard.rb
+++ b/lib/pro_tacts/vcard.rb
@@ -79,6 +79,14 @@ module ProTacts
def names?(name)
verbatim.match?(/\A(?:[A-Za-z0-9-]+\.)?#{Regexp.escape(name)}[;:]/i)
end
+
+ # Whether this line broke one of the parser's assumptions rather
+ # than merely failing to read. Asked here so no caller has to
+ # know the error taxonomy to tell the two apart.
+ #: () -> bool
+ def broke_assumption?
+ error.is_a?(Parser::BrokenAssumption)
+ end
end
# Text values escape backslash, the component separator, and the
@@ -180,6 +188,21 @@ module ProTacts
parsed && parsed.find { it.name.casecmp?("UID") }&.value
end
+ # The parser assumptions this card broke — the errors themselves,
+ # so a caller reporting them can say what was assumed and where.
+ # Empty for a card that reads, and for one that merely will not:
+ # a broken assumption is news about this server, ordinary bad
+ # input is not (Parser::BrokenAssumption). Deciding what to do
+ # about it is the caller's — reporting on every parse would fire
+ # on every read of the card, not once on its arrival.
+ #: () -> Array[Parser::BrokenAssumption]
+ def broken_assumptions
+ lines.filter_map { |line|
+ error = line.error
+ error if error.is_a?(Parser::BrokenAssumption)
+ }
+ end
+
# The card's logical lines, each parsed beside its verbatim bytes —
# the enumeration the questions that move bytes are asked over. A
# continuation travels with its line, terminators attached, so no
diff --git a/test/pro_tacts/vcard/test_lines.rb b/test/pro_tacts/vcard/test_lines.rb
index 1d4bd55..9683640 100644
--- a/test/pro_tacts/vcard/test_lines.rb
+++ b/test/pro_tacts/vcard/test_lines.rb
@@ -98,6 +98,29 @@ class VCardLinesTest < Minitest::Test
assert_equal "BDAY;=;:\r\n", unparsed.verbatim
end
+ # A broken assumption and an ordinary unreadable line are both
+ # errors on a line, and telling them apart belongs here rather than
+ # in whoever reports it: news about this server and ordinary bad
+ # input want different handling, and neither caller should have to
+ # know the parser's error taxonomy to pick.
+ def test_a_card_separates_broken_assumptions_from_bad_input
+ packed = VCARD.new("BEGIN:VCARD\r\nFN:A\rNOTE:n\r\nEND:VCARD\r\n")
+
+ assert_equal 1, packed.broken_assumptions.length
+ assert_match "bare CR", packed.broken_assumptions.fetch(0).message
+ assert_equal [false, true, false], packed.lines.map { it.broke_assumption? }
+
+ unreadable = VCARD.new("BEGIN:VCARD\r\nBDAY;=;:\r\nEND:VCARD\r\n")
+
+ assert_kind_of VCARD::ParseError, unreadable.lines.fetch(1).error
+ assert_empty unreadable.broken_assumptions
+ refute unreadable.lines.fetch(1).broke_assumption?
+ end
+
+ def test_a_card_that_reads_broke_no_assumption
+ assert_empty VCARD.new(CARD).broken_assumptions
+ end
+
## insert
def test_insert_takes_the_end_lines_terminator_for_a_bare_line