refactor!: streamline comparison/hashing functions (#755)

we have different methods of object comparison here and there, some work
on tuples, other on hashes, other on different structures.

this PR streamlines this.

these changes might cause breaking changes for downstream users.

---------

Signed-off-by: Jan Kowalleck <[email protected]>

Author: jkowalleck

cyclonedx/_internal/compare.py

@@ -58,40 +58,25 @@ def __gt__(self, other: Any) -> bool:
         return False
 
 
-class ComparableDict:
+class ComparableDict(ComparableTuple):
     """
     Allows comparison of dictionaries, allowing for missing/None values.
     """
 
-    def __init__(self, dict_: Dict[Any, Any]) -> None:
-        self._dict = dict_
-
-    def __lt__(self, other: Any) -> bool:
-        if not isinstance(other, ComparableDict):
-            return True
-        keys = sorted(self._dict.keys() | other._dict.keys())
-        return ComparableTuple(self._dict.get(k) for k in keys) \
-            < ComparableTuple(other._dict.get(k) for k in keys)
-
-    def __gt__(self, other: Any) -> bool:
-        if not isinstance(other, ComparableDict):
-            return False
-        keys = sorted(self._dict.keys() | other._dict.keys())
-        return ComparableTuple(self._dict.get(k) for k in keys) \
-            > ComparableTuple(other._dict.get(k) for k in keys)
+    def __new__(cls, d: Dict[Any, Any]) -> 'ComparableDict':
+        return super(ComparableDict, cls).__new__(cls, sorted(d.items()))
 
 
 class ComparablePackageURL(ComparableTuple):
     """
     Allows comparison of PackageURL, allowing for qualifiers.
     """
 
-    def __new__(cls, purl: 'PackageURL') -> 'ComparablePackageURL':
-        return super().__new__(
-            ComparablePackageURL, (
-                purl.type,
-                purl.namespace,
-                purl.version,
-                ComparableDict(purl.qualifiers) if isinstance(purl.qualifiers, dict) else purl.qualifiers,
-                purl.subpath
-            ))
+    def __new__(cls, p: 'PackageURL') -> 'ComparablePackageURL':
+        return super(ComparablePackageURL, cls).__new__(cls, (
+            p.type,
+            p.namespace,
+            p.version,
+            ComparableDict(p.qualifiers) if isinstance(p.qualifiers, dict) else p.qualifiers,
+            p.subpath
+        ))

cyclonedx/model/__init__.py

@@ -133,22 +133,23 @@ def classification(self) -> str:
     def classification(self, classification: str) -> None:
         self._classification = classification
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self.flow, self.classification
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, DataClassification):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __lt__(self, other: object) -> bool:
         if isinstance(other, DataClassification):
-            return _ComparableTuple((
-                self.flow, self.classification
-            )) < _ComparableTuple((
-                other.flow, other.classification
-            ))
+            return self.__comparable_tuple() < other.__comparable_tuple()
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash((self.flow, self.classification))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<DataClassification flow={self.flow}>'
@@ -236,22 +237,23 @@ def content(self) -> str:
     def content(self, content: str) -> None:
         self._content = content
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self.content_type, self.encoding, self.content,
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, AttachedText):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, AttachedText):
-            return _ComparableTuple((
-                self.content_type, self.content, self.encoding
-            )) < _ComparableTuple((
-                other.content_type, other.content, other.encoding
-            ))
+            return self.__comparable_tuple() < other.__comparable_tuple()
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash((self.content, self.content_type, self.encoding))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<AttachedText content-type={self.content_type}, encoding={self.encoding}>'
@@ -515,22 +517,23 @@ def content(self) -> str:
     def content(self, content: str) -> None:
         self._content = content
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self.alg, self.content
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, HashType):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, HashType):
-            return _ComparableTuple((
-                self.alg, self.content
-            )) < _ComparableTuple((
-                other.alg, other.content
-            ))
+            return self.__comparable_tuple() < other.__comparable_tuple()
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash((self.alg, self.content))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<HashType {self.alg.name}:{self.content}>'
@@ -733,7 +736,7 @@ def __init__(self, uri: str) -> None:
 
     def __eq__(self, other: Any) -> bool:
         if isinstance(other, XsUri):
-            return hash(other) == hash(self)
+            return self._uri == other._uri
         return False
 
     def __lt__(self, other: Any) -> bool:
@@ -892,25 +895,24 @@ def hashes(self) -> 'SortedSet[HashType]':
     def hashes(self, hashes: Iterable[HashType]) -> None:
         self._hashes = SortedSet(hashes)
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self._type, self._url, self._comment,
+            _ComparableTuple(self._hashes)
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, ExternalReference):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, ExternalReference):
-            return _ComparableTuple((
-                self._type, self._url, self._comment
-            )) < _ComparableTuple((
-                other._type, other._url, other._comment
-            ))
+            return self.__comparable_tuple() < other.__comparable_tuple()
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash((
-            self._type, self._url, self._comment,
-            tuple(sorted(self._hashes, key=hash))
-        ))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<ExternalReference {self.type.name}, {self.url}>'
@@ -969,22 +971,23 @@ def value(self) -> Optional[str]:
     def value(self, value: Optional[str]) -> None:
         self._value = value
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self.name, self.value
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, Property):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, Property):
-            return _ComparableTuple((
-                self.name, self.value
-            )) < _ComparableTuple((
-                other.name, other.value
-            ))
+            return self.__comparable_tuple() < other.__comparable_tuple()
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash((self.name, self.value))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<Property name={self.name}>'
@@ -1060,22 +1063,23 @@ def encoding(self) -> Optional[Encoding]:
     def encoding(self, encoding: Optional[Encoding]) -> None:
         self._encoding = encoding
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self.content, self.content_type, self.encoding
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, NoteText):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, NoteText):
-            return _ComparableTuple((
-                self.content, self.content_type, self.encoding
-            )) < _ComparableTuple((
-                other.content, other.content_type, other.encoding
-            ))
+            return self.__comparable_tuple() < other.__comparable_tuple()
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash((self.content, self.content_type, self.encoding))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<NoteText content_type={self.content_type}, encoding={self.encoding}>'
@@ -1144,22 +1148,23 @@ def locale(self, locale: Optional[str]) -> None:
                     " ISO-3166 (or higher) country code. according to ISO-639 format. Examples include: 'en', 'en-US'."
                 )
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self.locale, self.text
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, Note):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, Note):
-            return _ComparableTuple((
-                self.locale, self.text
-            )) < _ComparableTuple((
-                other.locale, other.text
-            ))
+            return self.__comparable_tuple() < other.__comparable_tuple()
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash((self.text, self.locale))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<Note id={id(self)}, locale={self.locale}>'
@@ -1234,22 +1239,23 @@ def email(self) -> Optional[str]:
     def email(self, email: Optional[str]) -> None:
         self._email = email
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self.timestamp, self.name, self.email
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, IdentifiableAction):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, IdentifiableAction):
-            return _ComparableTuple((
-                self.timestamp, self.name, self.email
-            )) < _ComparableTuple((
-                other.timestamp, other.name, other.email
-            ))
+            return self.__comparable_tuple() < other.__comparable_tuple()
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash((self.timestamp, self.name, self.email))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<IdentifiableAction name={self.name}, email={self.email}>'
@@ -1287,16 +1293,16 @@ def text(self, text: str) -> None:
 
     def __eq__(self, other: object) -> bool:
         if isinstance(other, Copyright):
-            return hash(other) == hash(self)
+            return self._text == other._text
         return False
 
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, Copyright):
-            return self.text < other.text
+            return self._text < other._text
         return NotImplemented
 
     def __hash__(self) -> int:
-        return hash(self.text)
+        return hash(self._text)
 
     def __repr__(self) -> str:
         return f'<Copyright text={self.text}>'

cyclonedx/model/bom.py

@@ -25,6 +25,7 @@
 import serializable
 from sortedcontainers import SortedSet
 
+from .._internal.compare import ComparableTuple as _ComparableTuple
 from .._internal.time import get_now_utc as _get_now_utc
 from ..exception.model import LicenseExpressionAlongWithOthersException, UnknownComponentDependencyException
 from ..schema.schema import (
@@ -293,16 +294,20 @@ def properties(self) -> 'SortedSet[Property]':
     def properties(self, properties: Iterable[Property]) -> None:
         self._properties = SortedSet(properties)
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            _ComparableTuple(self.authors), self.component, _ComparableTuple(self.licenses), self.manufacture,
+            _ComparableTuple(self.properties),
+            _ComparableTuple(self.lifecycles), self.supplier, self.timestamp, self.tools, self.manufacturer
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, BomMetaData):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __hash__(self) -> int:
-        return hash((
-            tuple(self.authors), self.component, tuple(self.licenses), self.manufacture, tuple(self.properties),
-            tuple(self.lifecycles), self.supplier, self.timestamp, self.tools, self.manufacturer
-        ))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<BomMetaData timestamp={self.timestamp}, component={self.component}>'
@@ -722,17 +727,22 @@ def validate(self) -> bool:
 
         return True
 
+    def __comparable_tuple(self) -> _ComparableTuple:
+        return _ComparableTuple((
+            self.serial_number, self.version, self.metadata, _ComparableTuple(
+                self.components), _ComparableTuple(self.services),
+            _ComparableTuple(self.external_references), _ComparableTuple(
+                self.dependencies), _ComparableTuple(self.properties),
+            _ComparableTuple(self.vulnerabilities),
+        ))
+
     def __eq__(self, other: object) -> bool:
         if isinstance(other, Bom):
-            return hash(other) == hash(self)
+            return self.__comparable_tuple() == other.__comparable_tuple()
         return False
 
     def __hash__(self) -> int:
-        return hash((
-            self.serial_number, self.version, self.metadata, tuple(self.components), tuple(self.services),
-            tuple(self.external_references), tuple(self.dependencies), tuple(self.properties),
-            tuple(self.vulnerabilities),
-        ))
+        return hash(self.__comparable_tuple())
 
     def __repr__(self) -> str:
         return f'<Bom uuid={self.serial_number}, hash={hash(self)}>'