jdb/nim-vcard
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor vCard validation and canonicalize VERSION handling

Browse Source 
Move whole-card cardinality checks into overloaded validate helpers in
the vCard 3 and vCard 4 modules, and have readVCard invoke those
validators after parsing instead of hard-coding property checks in the
shared parser.

For vCard 3, validate property cardinality directly from propertyCardMap
and add regression coverage for duplicate single-cardinality properties.
For vCard 4, validate from propertyCardMap as well, while preserving
ALTID semantics for single-cardinality properties so alternate
representations remain valid and duplicate ungrouped properties are
rejected.

Rework VERSION handling to treat it as parser metadata rather than
stored card content. Parsing still requires exactly one canonical
VERSION line and rejects missing or duplicate occurrences, but parsed
cards no longer retain VC3_Version or VC4_Version objects in content.
Accessors and serializers continue to expose and emit the canonical
version for the concrete card type, which keeps parsed and
programmatically constructed cards on the same model.

Update the vCard 3 and vCard 4 test suites to cover the new validation
path, confirm VERSION is not persisted in content, and adjust fixture
expectations to match the canonical VERSION model.

AI-Assisted: yes
AI-Tool: OpenAI Codex / gpt-5.4 xhigh
This commit is contained in:
Jonathan Bernard
2026-03-28 19:25:45 -05:00
parent cfac536d60
commit e3f0fdc1ab
5 changed files with 139 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/vcard.nim
View File

@@ -55,6 +55,14 @@ proc readVCard*(p: var VCardParser): VCard =
if result.parsedVersion == VCardV3:
while (p.skip(CRLF, true)): discard
try:
if result.parsedVersion == VCardV3:
cast[VCard3](result).validate()
else:
cast[VCard4](result).validate()
except ValueError as exc:
p.error(exc.msg)
proc initVCardParser*(input: Stream, filename = "input"): VCardParser =
result.filename = filename
lexer.open(result, input)

45
src/vcard/vcard3.nim
View File

@@ -707,18 +707,39 @@ genPropertyAccessors(propertyCardMap.pairs.toSeq -->
filter(not [pnVersion].contains(it[0])))
func version*(vc3: VCard3): VC3_Version =
## Return the VERSION property.
let found = findFirst[VC3_Version, VC3_Property](vc3.content)
if found.isSome: return found.get
else: return VC3_Version(
propertyId: vc3.content.len + 1,
group: none[string](),
name: "VERSION",
value: "3.0")
## Return the canonical VERSION property for a vCard 3.0 card.
result = newVC3_Version()
func xTypes*(vc3: VCard3): seq[VC3_XType] = findAll[VC3_XType, VC3_Property](vc3.content)
## Return all extended properties (starting with `x-`).
func validate*(vc3: VCard3): void =
## Validate property cardinality for a vCard 3.0 card.
for pn in VC3_PropertyName:
if pn == pnVersion:
continue
let count = vc3.content.countIt(it.name == $pn)
case propertyCardMap[pn]
of vpcExactlyOne:
if count != 1:
raise newException(ValueError,
"VCard should have exactly one $# property, but $# were found" %
[$pn, $count])
of vpcAtLeastOne:
if count < 1:
raise newException(ValueError,
"VCard should have at least one $# property, but $# were found" %
[$pn, $count])
of vpcAtMostOne:
if count > 1:
raise newException(ValueError,
"VCard should have at most one $# property, but $# were found" %
[$pn, $count])
of vpcAny:
discard
# Setters
# =============================================================================
@@ -1030,6 +1051,7 @@ proc readBinaryValue(
proc parseContentLines*(p: var VCardParser): seq[VC3_Property] =
result = @[]
var sawVersion = false
macro assignCommon(assign: untyped): untyped =
result = assign
@@ -1271,9 +1293,11 @@ proc parseContentLines*(p: var VCardParser): seq[VC3_Property] =
result.add(newVC3_URL(group = group, value = p.readValue))
of $pnVersion:
if sawVersion:
p.error("VCard should have exactly one VERSION property, but 2 were found")
p.expect("3.0")
p.validateNoParameters(params, "VERSION")
result.add(newVC3_Version(group = group))
sawVersion = true
of $pnClass:
result.add(newVC3_Class(group = group, value = p.readValue))
@@ -1312,6 +1336,9 @@ proc parseContentLines*(p: var VCardParser): seq[VC3_Property] =
p.expect("\r\n")
if not sawVersion:
p.error("VCard should have exactly one VERSION property, but 0 were found")
#[
Simplified Parsing Diagram

63
src/vcard/vcard4.nim
View File

@@ -968,6 +968,56 @@ genPidAccessors(supportedParams["PID"].toSeq())
genPrefAccessors(supportedParams["PREF"].toSeq())
genTypeAccessors(supportedParams["TYPE"].toSeq())
func countCardinalityInstances(vc4: VCard4, propName: string): int =
var altIds = initHashSet[string]()
for p in vc4.content:
if p.name != propName:
continue
if p.altId.isSome:
altIds.incl(p.altId.get)
else:
inc result
result += altIds.len
func validate*(vc4: VCard4): void =
## Validate property cardinality for a vCard 4.0 card.
for pn in VC4_PropertyName:
if pn == pnVersion:
continue
if not propertyCardMap.contains(pn):
continue
let rawCount = vc4.content.countIt(it.name == $pn)
let cardinalityCount =
case propertyCardMap[pn]
of vpcExactlyOne, vpcAtMostOne:
vc4.countCardinalityInstances($pn)
of vpcAtLeastOne, vpcAny:
rawCount
case propertyCardMap[pn]
of vpcExactlyOne:
if cardinalityCount != 1:
raise newException(ValueError,
"VCard should have exactly one $# property, but $# were found" %
[$pn, $cardinalityCount])
of vpcAtLeastOne:
if cardinalityCount < 1:
raise newException(ValueError,
"VCard should have at least one $# property, but $# were found" %
[$pn, $cardinalityCount])
of vpcAtMostOne:
if cardinalityCount > 1:
raise newException(ValueError,
("VCard should have at most one $# property, but $# " &
"distinct properties were found") % [$pn, $cardinalityCount])
of vpcAny:
discard
# Setters
# =============================================================================
@@ -1382,6 +1432,7 @@ macro genPropParsers(
proc parseContentLines*(p: var VCardParser): seq[VC4_Property] =
result = @[]
var sawVersion = false
while true:
let group = p.readGroup
@@ -1392,10 +1443,20 @@ proc parseContentLines*(p: var VCardParser): seq[VC4_Property] =
let params = p.readParams
p.expect(":")
genPropParsers(fixedValueTypeProperties, group, name, params, result, p)
if name == $pnVersion:
if sawVersion:
p.error("VCard should have exactly one VERSION property, but 2 were found")
p.validateType(params, vtText)
p.expect("4.0")
sawVersion = true
else:
genPropParsers(fixedValueTypeProperties, group, name, params, result, p)
p.expect(CRLF)
if not sawVersion:
p.error("VCard should have exactly one VERSION property, but 0 were found")
# Private Function Unit Tests
# ============================================================================

23
tests/tvcard3.nim
View File

@@ -1,4 +1,4 @@
import std/[options, strutils, times, unittest]
import std/[options, sequtils, strutils, times, unittest]
import zero_functional
import vcard
@@ -57,6 +57,7 @@ suite "vcard/vcard3":
"BEGIN:vCard\r\n" &
"VERSION:3.0\r\n" &
"FN:Frank Dawson\r\n" &
"N:Dawson;Frank;;;\r\n" &
"ORG:Lotus Development Corporation\r\n" &
"ADR;TYPE=WORK,POSTAL,PARCEL:;;6544 Battleford Drive\r\n" &
" ;Raleigh;NC;27613-3502;U.S.A.\r\n" &
@@ -71,6 +72,7 @@ suite "vcard/vcard3":
"BEGIN:vCard\r\n" &
"VERSION:3.0\r\n" &
"FN:Tim Howes\r\n" &
"N:Howes;Tim;;;\r\n" &
"ORG:Netscape Communications Corp.\r\n" &
"ADR;TYPE=WORK:;;501 E. Middlefield Rd.;Mountain View;\r\n" &
" CA; 94043;U.S.A.\r\n" &
@@ -104,6 +106,25 @@ suite "vcard/vcard3":
"VERSION:3.0",
"FN:John Smith"))
test "spec: parser rejects duplicate single-cardinality vCard 3 properties":
expect(VCardParsingError):
discard parseVCards(vcard3Doc(
"VERSION:3.0",
"FN:John Smith",
"N:Smith;John;;;",
"BDAY:2000-01-01",
"BDAY:2000-01-02"))
test "spec: VERSION is not stored as vCard 3 content":
let parsed = parseSingleVCard3(vcard3Doc(
"VERSION:3.0",
"FN:John Smith",
"N:Smith;John;;;"))
check:
parsed.version.value == "3.0"
parsed.content.countIt(it of VC3_Version) == 0
test "spec: simple text values decode RFC 2426 escapes when parsing":
let parsed = parseSingleVCard3(vcard3Doc(
"VERSION:3.0",

12
tests/tvcard4.nim
View File

@@ -1,4 +1,4 @@
import std/[options, strutils, tables, unittest]
import std/[options, sequtils, strutils, tables, unittest]
import zero_functional
import vcard
@@ -203,6 +203,7 @@ suite "vcard/vcard4":
madeUpProp.value == "Sample value for my made-up prop."
let cardWithAltBdayStr = testVCardTemplate % [(
"FN:Simon Perreault\r\n" &
"BDAY;VALUE=text;ALTID=1:20th century\r\n" &
"BDAY;VALUE=date-and-or-time;ALTID=1:19650321\r\n"
)]
@@ -210,6 +211,14 @@ suite "vcard/vcard4":
test "single-cardinality properties allow multiples with ALTID":
check parseVCards(cardWithAltBdayStr).len == 1
test "single-cardinality properties reject multiples without ALTID":
expect(VCardParsingError):
discard parseVCards(testVCardTemplate % [(
"FN:Simon Perreault\r\n" &
"BDAY;VALUE=text:20th century\r\n" &
"BDAY;VALUE=date-and-or-time:19650321\r\n"
)])
let hasAltBdays = cast[VCard4](parseVCards(cardWithAltBdayStr)[0])
test "properties with cardinality 1 and altids return the first found by default":
@@ -222,6 +231,7 @@ suite "vcard/vcard4":
check:
hasAltBdays.content.len == 3
hasAltBdays.bday.isSome
hasAltBdays.content.countIt(it of VC4_Version) == 0
let allBdays = allAlternatives[VC4_Bday](hasAltBdays)
check: