diff --git a/khard/contacts.py b/khard/contacts.py index 159cda4..9752bba 100644 --- a/khard/contacts.py +++ b/khard/contacts.py @@ -39,6 +39,7 @@ logger = logging.getLogger(__name__) T = TypeVar("T") +LabeledStrs = list[Union[str, dict[str, str]]] @overload @@ -112,12 +113,19 @@ def __init__(self, vcard: vobject.base.Component, def __str__(self) -> str: return self.formatted_name - def get_first(self, property: str, default: str = "") -> str: + @overload + def get_first(self, property: Literal["n"]) -> Optional[vobject.vcard.Name]: ... + @overload + def get_first(self, property: Literal["adr"]) -> Optional[vobject.vcard.Address]: ... + @overload + def get_first(self, property: str) -> Optional[str]: ... + def get_first(self, property: str) -> Union[None, str, vobject.vcard.Name, + vobject.vcard.Address]: """Get a property from the underlying vCard. This method should only be called for properties with cardinality \\*1 (zero or one). Otherwise only the first value will be returned. If the - property is not present a default will be returned. + property is not present None will be retuned. The type annotation for the return value is str but this is not enforced so it is up to the caller to make sure to only call this @@ -125,17 +133,15 @@ def get_first(self, property: str, default: str = "") -> str: str. :param property: the field value to get - :param default: the value to return if the vCard does not have this - property - :returns: the property value or the default + :returns: the property value or None """ try: return getattr(self.vcard, property).value except AttributeError: - return default + return None - def _get_multi_property(self, name: str) -> list: - """Get a vCard property that can exist more than once. + def get_all(self, name: str) -> list: + """Get all values of the given vCard property. It does not matter what the individual vcard properties store as their value. This function returns them untouched inside an aggregating @@ -147,14 +153,12 @@ def _get_multi_property(self, name: str) -> list: :param name: the name of the property (should be UPPER case) :returns: the values from all occurrences of the named property """ - values = [] - for child in self.vcard.getChildren(): - if child.name == name: - ablabel = self._get_ablabel(child) - if ablabel: - values.append({ablabel: child.value}) - else: - values.append(child.value) + try: + values = getattr(self.vcard, f"{name.lower()}_list") + except AttributeError: + return [] + values = [{label: item.value} if (label := self._get_ablabel(item)) + else item.value for item in values] return sorted(values, key=multi_property_key) def _delete_vcard_object(self, name: str) -> None: @@ -259,7 +263,7 @@ def _get_types_for_vcard_object(self, object: vobject.base.ContentLine, return [default_type] @property - def version(self) -> str: + def version(self) -> Optional[str]: return self.get_first("version") @version.setter @@ -274,7 +278,7 @@ def version(self, value: str) -> None: version.value = convert_to_vcard("version", value, ObjectType.str) @property - def uid(self) -> str: + def uid(self) -> Optional[str]: return self.get_first("uid") @uid.setter @@ -470,7 +474,7 @@ def _prepare_birthday_value(self, date: Date) -> tuple[Optional[str], @property def kind(self) -> str: - return self.get_first(self._kind_attribute_name().lower(), self._default_kind) + return self.get_first(self._kind_attribute_name().lower()) or self._default_kind @kind.setter def kind(self, value: str) -> None: @@ -487,10 +491,15 @@ def _kind_attribute_name(self) -> str: @property def formatted_name(self) -> str: - return self.get_first("fn") + fn = self.get_first("fn") + if fn: + return fn + self.formatted_name = "" + return self.get_first("fn") or "" @formatted_name.setter def formatted_name(self, value: str) -> None: + # TODO cardinality 1* """Set the FN field to the new value. All previously existing FN fields are deleted. Version 4 of the specs @@ -522,10 +531,9 @@ def _get_names_part(self, part: str) -> list[str]: the_list = getattr(self.vcard.n.value, part) except AttributeError: return [] - else: - # check if list only contains empty strings - if not ''.join(the_list): - return [] + # check if list only contains empty strings + if not ''.join(the_list): + return [] return the_list if isinstance(the_list, list) else [the_list] def _get_name_prefixes(self) -> list[str]: @@ -573,12 +581,16 @@ def get_last_name_first_name(self) -> str: return self.formatted_name @property - def first_name(self) -> str: - return list_to_string(self._get_first_names(), " ") + def first_name(self) -> Optional[str]: + if parts := self._get_first_names(): + return list_to_string(parts, " ") + return None @property - def last_name(self) -> str: - return list_to_string(self._get_last_names(), " ") + def last_name(self) -> Optional[str]: + if parts := self._get_last_names(): + return list_to_string(parts, " ") + return None def _add_name(self, prefix: StrList, first_name: StrList, additional_name: StrList, last_name: StrList, @@ -605,7 +617,7 @@ def organisations(self) -> list[Union[list[str], dict[str, list[str]]]]: """ :returns: list of organisations, sorted alphabetically """ - return self._get_multi_property("ORG") + return self.get_all("org") def _add_organisation(self, organisation: StrList, label: Optional[str] = None) -> None: """Add one ORG entry to the underlying vcard @@ -628,48 +640,47 @@ def _add_organisation(self, organisation: StrList, label: Optional[str] = None) showas_obj.value = "COMPANY" @property - def titles(self) -> list[Union[str, dict[str, str]]]: - return self._get_multi_property("TITLE") + def titles(self) -> LabeledStrs: + return self.get_all("title") def _add_title(self, title: str, label: Optional[str] = None) -> None: self._add_labelled_property("title", title, label, True) @property - def roles(self) -> list[Union[str, dict[str, str]]]: - return self._get_multi_property("ROLE") + def roles(self) -> LabeledStrs: + return self.get_all("role") def _add_role(self, role: str, label: Optional[str] = None) -> None: self._add_labelled_property("role", role, label, True) @property - def nicknames(self) -> list[Union[str, dict[str, str]]]: - return self._get_multi_property("NICKNAME") + def nicknames(self) -> LabeledStrs: + return self.get_all("nickname") def _add_nickname(self, nickname: str, label: Optional[str] = None) -> None: self._add_labelled_property("nickname", nickname, label, True) @property - def notes(self) -> list[Union[str, dict[str, str]]]: - return self._get_multi_property("NOTE") + def notes(self) -> LabeledStrs: + return self.get_all("note") def _add_note(self, note: str, label: Optional[str] = None) -> None: self._add_labelled_property("note", note, label, True) @property - def webpages(self) -> list[Union[str, dict[str, str]]]: - return self._get_multi_property("URL") + def webpages(self) -> LabeledStrs: + return self.get_all("url") def _add_webpage(self, webpage: str, label: Optional[str] = None) -> None: self._add_labelled_property("url", webpage, label, True) @property def categories(self) -> Union[list[str], list[list[str]]]: - category_list = [] - for child in self.vcard.getChildren(): - if child.name == "CATEGORIES": - value = child.value - category_list.append( - value if isinstance(value, list) else [value]) + category_list = self.get_all("categories") + if not category_list: + return category_list + category_list = [value if isinstance(value, list) else [value] for + value in category_list] if len(category_list) == 1: return category_list[0] return sorted(category_list) @@ -754,13 +765,16 @@ def emails(self) -> dict[str, list[str]]: :returns: dict of type and email address list """ email_dict: dict[str, list[str]] = {} - for child in self.vcard.getChildren(): - if child.name == "EMAIL": - type = list_to_string( - self._get_types_for_vcard_object(child, "internet"), ", ") - if type not in email_dict: - email_dict[type] = [] - email_dict[type].append(child.value) + try: + emails = self.vcard.email_list + except AttributeError: + return {} + for child in emails: + type = list_to_string( + self._get_types_for_vcard_object(child, "internet"), ", ") + if type not in email_dict: + email_dict[type] = [] + email_dict[type].append(child.value) # sort email address lists for email_list in email_dict.values(): email_list.sort() @@ -805,19 +819,22 @@ def post_addresses(self) -> dict[str, list[PostAddress]]: :returns: dict of type and post address list """ post_adr_dict: dict[str, list[PostAddress]] = {} - for child in self.vcard.getChildren(): - if child.name == "ADR": - type = list_to_string(self._get_types_for_vcard_object( - child, "home"), ", ") - if type not in post_adr_dict: - post_adr_dict[type] = [] - post_adr_dict[type].append({"box": child.value.box, - "extended": child.value.extended, - "street": child.value.street, - "code": child.value.code, - "city": child.value.city, - "region": child.value.region, - "country": child.value.country}) + try: + addresses = self.vcard.adr_list + except AttributeError: + return {} + for child in addresses: + type = list_to_string(self._get_types_for_vcard_object( + child, "home"), ", ") + if type not in post_adr_dict: + post_adr_dict[type] = [] + post_adr_dict[type].append({"box": child.value.box, + "extended": child.value.extended, + "street": child.value.street, + "code": child.value.code, + "city": child.value.city, + "region": child.value.region, + "country": child.value.country}) # sort post address lists for post_adr_list in post_adr_dict.values(): post_adr_list.sort(key=lambda x: ( @@ -930,9 +947,9 @@ def __init__(self, vcard: vobject.base.Component, # getters and setters ##################### - def _get_private_objects(self) -> dict[str, list[Union[str, dict[str, str]]]]: + def _get_private_objects(self) -> dict[str, LabeledStrs]: supported = [x.lower() for x in self.supported_private_objects] - private_objects: dict[str, list[Union[str, dict[str, str]]]] = {} + private_objects: dict[str, LabeledStrs] = {} for child in self.vcard.getChildren(): lower = child.name.lower() if lower.startswith("x-") and lower[2:] in supported: @@ -1303,9 +1320,11 @@ def to_yaml(self) -> str: "Note": self.notes, "Webpage": self.webpages, "Anniversary": - helpers.yaml_anniversary(self.anniversary, self.version), + helpers.yaml_anniversary(self.anniversary, self.version or + self._default_version), "Birthday": - helpers.yaml_anniversary(self.birthday, self.version), + helpers.yaml_anniversary(self.birthday, self.version or + self._default_version), "Address": helpers.yaml_addresses( self.post_addresses, ["Box", "Extended", "Street", "Code", "City", "Region", "Country"], defaults=["home"]) diff --git a/khard/khard.py b/khard/khard.py index 109defc..bc5bc24 100644 --- a/khard/khard.py +++ b/khard/khard.py @@ -210,9 +210,9 @@ def list_contacts(vcard_list: list[Contact], fields: Iterable[str] = (), row.append(formatter.get_special_field(vcard, field)) elif field == 'uid': if parsable: - row.append(vcard.uid) - elif abook_collection.get_short_uid(vcard.uid): - row.append(abook_collection.get_short_uid(vcard.uid)) + row.append(vcard.uid or "") + elif uid := abook_collection.get_short_uid(vcard.uid or ""): + row.append(uid) else: row.append("") else: diff --git a/test/test_query.py b/test/test_query.py index f1d45bd..6b0133e 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -105,11 +105,7 @@ def test_and_queries_match_after_sorting(self): class TestFieldQuery(unittest.TestCase): - @unittest.expectedFailure - def test_empty_field_values_match_if_the_field_is_present(self): - # This test currently fails because the Contact class has all - # attributes set because they are properties. So the test in the query - # class if an attribute is present never fails. + def test_empty_field_values_match_if_sstring_field_is_present(self): uid = "Some Test Uid" vcard1 = TestContact(uid=uid) vcard2 = TestContact() @@ -117,6 +113,20 @@ def test_empty_field_values_match_if_the_field_is_present(self): self.assertTrue(query.match(vcard1)) self.assertFalse(query.match(vcard2)) + def test_empty_field_values_match_if_list_field_is_present(self): + vcard1 = TestContact(categories=["foo", "bar"]) + vcard2 = TestContact() + query = FieldQuery("categories", "") + self.assertTrue(query.match(vcard1)) + self.assertFalse(query.match(vcard2)) + + def test_empty_field_values_match_if_dict_field_is_present(self): + query = FieldQuery("emails", "") + vcard = TestContact() + self.assertFalse(query.match(vcard)) + vcard.add_email("home", "a@b.c") + self.assertTrue(query.match(vcard)) + def test_empty_field_values_fails_if_the_field_is_absent(self): vcard = TestContact() query = FieldQuery("emails", "") diff --git a/test/test_vcard_wrapper.py b/test/test_vcard_wrapper.py index 19d98f3..79938cd 100644 --- a/test/test_vcard_wrapper.py +++ b/test/test_vcard_wrapper.py @@ -8,7 +8,7 @@ import vobject -from khard.contacts import VCardWrapper +from khard.contacts import Contact, VCardWrapper from khard.helpers.typing import ObjectType from .helpers import vCard, TestVCardWrapper @@ -510,7 +510,7 @@ class AddLabelledObject(unittest.TestCase): def assertTitle(self, expected): wrapper = TestVCardWrapper() yield wrapper - self.assertEqual(wrapper._get_multi_property("TITLE"), expected) + self.assertEqual(wrapper.get_all("title"), expected) def test_add_a_string(self): with self.assertTitle(["foo"]) as wrapper: @@ -564,8 +564,7 @@ def test_get_only_the_first_property(self): def test_returns_the_default(self): wrapper = TestVCardWrapper() - self.assertEqual(wrapper.get_first("title"), "") - self.assertEqual(wrapper.get_first("title", "foo"), "foo") + self.assertIsNone(wrapper.get_first("title")) def test_can_return_any_value_contradicting_type_annotation(self): """This is discouraged!""" @@ -574,3 +573,38 @@ def test_can_return_any_value_contradicting_type_annotation(self): p.value = vobject.vcard.Name(family='Foo', given='Bar') self.assertEqual(wrapper.get_first("n"), vobject.vcard.Name(family='Foo', given='Bar')) + + +class NullableProperties(unittest.TestCase): + "test that attributes that are not present on the vcard return None" + + LIST_PROPERTIES = ["categories", "titles", "webpages", "organisations", + "notes", "roles", "nicknames"] + DICT_PROPERTIES = ["post_addresses", "emails", "phone_numbers"] + BASE_PROPERTIES = ["formatted_name", "kind", "version"] + + def test_for_non_existing_attributes(self): + """Non existing attributes""" + for version in ["3.0", "4.0"]: + card = TestVCardWrapper(version=version) + for property in Contact.get_properties(): + if property in self.DICT_PROPERTIES: + with self.subTest(property=property, version=version): + self.assertEqual(getattr(card, property), {}) + elif property in self.LIST_PROPERTIES: + with self.subTest(property=property, version=version): + self.assertEqual(getattr(card, property), []) + elif property not in self.BASE_PROPERTIES: + with self.subTest(property=property, version=version): + self.assertIsNone(getattr(card, property)) + + @unittest.expectedFailure + def test_no_name_is_not_equal_to_empty_name(self): + # FIXME this fails because khard.contacts.VCardWrapper._get_names_part + # specifically treats a name where all components are the empty string + # the same way as no N attribute at all. + empty = TestVCardWrapper() + empty._add_name("", "", "", "", "") + noname = TestVCardWrapper() + self.assertNotEqual(empty.first_name, noname.first_name) + self.assertNotEqual(empty.last_name, noname.last_name)