kenianbei / vcard_parser

Parses and validates vCard data according to RFC 6350 specification.
MIT License
6 stars 3 forks source link

Suggestion about UUID usage #8

Closed tmpfs closed 1 year ago

tmpfs commented 2 years ago

I just round-tripped a vCard to a string and back to a Vcard but they are not considered equal by assert_eq! which is because of the UUID generation I believe.

I wonder if using UUIDs is a good idea and whether there is a more ergonomic API for accessing properties.

I would imagine more of a HashMap style API where if a property accepts multiple values it is a Vec<Value> (otherwise just Value), then we could look up by property identifier as opposed to UUID, something like: get_property("full_name"). What do you think?

kenianbei commented 2 years ago

My concerns with switching to a hashmap for properties is that 1) it becomes more difficult to convert all properties to an iterator and 2) with using a uuid it's easier to store a single uuid than having to store the hashmap index and the vec index for retrieving, updating, and removing properties.

For example if the hashmap "ADR" has an item added to the properties vec at the beginning of the vec, you would then need to update your reference to which index points to that specific property in your app.

Let me know if I'm misunderstanding your idea...

Also, it may be useful to impl Eq and PartialEq for vcards that ignore uuids. But they would still have to match vec index order I think.

tmpfs commented 2 years ago

That makes sense, thanks for clarifying. I think implementing Eq and PartialEq is the better approach in that case.

kenianbei commented 2 years ago

I've been thinking about this more, and the better solution is to fully implement PID and CLIENTPIDMAP. Right now the library doesn't worry about matching properties or syncing two vcards.

Relevant section: https://www.rfc-editor.org/rfc/rfc6350#section-7.1.3

My understanding from the spec is that CLIENTPIDMAP will refer to the source device or application instance that is editing the vCard, and each PID will have two digits separated by a period. The first digit is the property ID, which can be used to match the property, and the second digit refers to the CLIENTPIDMAP, which can be used for syncing.

I may take a crack at this on the weekend, but it's going to be complicated. I think adding a sync module with tests and test data taken from the spec will be the way to get this coded.

kenianbei commented 1 year ago

I completely rewrote a big chunk of this library to use PIDs for matching, it should mostly follow the spec now.