Fix backup equal and add hash to objects with eq (#6059)

* Fix backup equal and add hash to objects with eq

* Add test for failed consolidate

supervisor/backups/backup.py

@@ -262,41 +262,35 @@ class Backup(JobGroup):
 
     def __eq__(self, other: Any) -> bool:
         """Return true if backups have same metadata."""
-        if not isinstance(other, Backup):
+        return isinstance(other, Backup) and self.slug == other.slug
-            return False
 
-        # Compare all fields except ones about protection. Current encryption status does not affect equality
+    def __hash__(self) -> int:
-        keys = self._data.keys() | other._data.keys()
+        """Return hash of backup."""
-        for k in keys - IGNORED_COMPARISON_FIELDS:
+        return hash(self.slug)
-            if (
-                k not in self._data
-                or k not in other._data
-                or self._data[k] != other._data[k]
-            ):
-                _LOGGER.info(
-                    "Backup %s and %s not equal because %s field has different value: %s and %s",
-                    self.slug,
-                    other.slug,
-                    k,
-                    self._data.get(k),
-                    other._data.get(k),
-                )
-                return False
-        return True
 
     def consolidate(self, backup: Self) -> None:
         """Consolidate two backups with same slug in different locations."""
-        if self.slug != backup.slug:
+        if self != backup:
             raise ValueError(
                 f"Backup {self.slug} and {backup.slug} are not the same backup"
             )
-        if self != backup:
+
-            raise BackupInvalidError(
+        # Compare all fields except ones about protection. Current encryption status does not affect equality
-                f"Backup in {backup.location} and {self.location} both have slug {self.slug} but are not the same!"
+        other_data = backup._data  # pylint: disable=protected-access
-            )
+        keys = self._data.keys() | other_data.keys()
+        for k in keys - IGNORED_COMPARISON_FIELDS:
+            if (
+                k not in self._data
+                or k not in other_data
+                or self._data[k] != other_data[k]
+            ):
+                raise BackupInvalidError(
+                    f"Cannot consolidate backups in {backup.location} and {self.location} with slug {self.slug} "
+                    f"because field {k} has different values: {self._data.get(k)} and {other_data.get(k)}!",
+                    _LOGGER.error,
+                )
 
         # In case of conflict we always ignore the ones from the first one. But log them to let the user know
-
         if conflict := {
             loc: val.path
             for loc, val in self.all_locations.items()

supervisor/mounts/mount.py

@@ -164,10 +164,14 @@ class Mount(CoreSysAttributes, ABC):
         """Return true if successfully mounted and available."""
         return self.state == UnitActiveState.ACTIVE
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Return true if mounts are the same."""
         return isinstance(other, Mount) and self.name == other.name
 
+    def __hash__(self) -> int:
+        """Return hash of mount."""
+        return hash(self.name)
+
     async def load(self) -> None:
         """Initialize object."""
         # If there's no mount unit, mount it to make one

tests/backups/test_backup.py

@@ -185,6 +185,33 @@ async def test_consolidate(
     }
 
 
+@pytest.mark.usefixtures("tmp_supervisor_data")
+async def test_consolidate_failure(coresys: CoreSys, tmp_path: Path):
+    """Test consolidate with two backups that are not the same."""
+    (mount_dir := coresys.config.path_mounts / "backup_test").mkdir()
+    tar1 = Path(copy(get_fixture_path("test_consolidate_unc.tar"), tmp_path))
+    backup1 = Backup(coresys, tar1, "test", None)
+    await backup1.load()
+
+    tar2 = Path(copy(get_fixture_path("backup_example.tar"), mount_dir))
+    backup2 = Backup(coresys, tar2, "test", "backup_test")
+    await backup2.load()
+
+    with pytest.raises(
+        ValueError,
+        match=f"Backup {backup1.slug} and {backup2.slug} are not the same backup",
+    ):
+        backup1.consolidate(backup2)
+
+    # Force slugs to be the same to run the fields check
+    backup1._data["slug"] = backup2.slug  # pylint: disable=protected-access
+    with pytest.raises(
+        BackupInvalidError,
+        match=f"Cannot consolidate backups in {backup2.location} and {backup1.location} with slug {backup1.slug}",
+    ):
+        backup1.consolidate(backup2)
+
+
 @pytest.mark.parametrize(
     (
         "tarfile_side_effect",