Refactor _get_multi_property into get_all; make name getters nullable

The _list attribute on the underlying vcard object should be faster than
iterating through all attributes ourself.

With this it should be possible to detect if an attribute is present on
the vcard or not.  If it is not present the @Property on the wrapper
returns None or an empty list or an empty dict, depending on the
attribute.

Author: lucc

khard/contacts.py

@@ -39,6 +39,7 @@
 
 logger = logging.getLogger(__name__)
 T = TypeVar("T")
+LabeledStrs = list[Union[str, dict[str, str]]]
 
 
 @overload
@@ -132,17 +133,15 @@ def get_first(self, property: str) -> Union[None, str, vobject.vcard.Name,
         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 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
@@ -154,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:
@@ -585,12 +582,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,
@@ -617,7 +618,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
@@ -640,48 +641,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)
@@ -766,13 +766,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()
@@ -817,19 +820,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: (
@@ -942,9 +948,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:

test/test_vcard_wrapper.py

@@ -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:
@@ -578,10 +578,21 @@ def test_can_return_any_value_contradicting_type_annotation(self):
 class NullableProperties(unittest.TestCase):
     "test that properties 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_properties(self):
         for version in ["3.0", "4.0"]:
             card = TestVCardWrapper(version=version)
             for property in Contact.get_properties():
-                if property not in ["formatted_name", "version", "kind"]:
+                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))