Skip to content

Commit 0338964

Browse files
authored
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
1 parent 478e00c commit 0338964

File tree

3 files changed

+52
-27
lines changed

3 files changed

+52
-27
lines changed

supervisor/backups/backup.py

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -262,41 +262,35 @@ def data(self) -> dict[str, Any]:
262262

263263
def __eq__(self, other: Any) -> bool:
264264
"""Return true if backups have same metadata."""
265-
if not isinstance(other, Backup):
266-
return False
265+
return isinstance(other, Backup) and self.slug == other.slug
267266

268-
# Compare all fields except ones about protection. Current encryption status does not affect equality
269-
keys = self._data.keys() | other._data.keys()
270-
for k in keys - IGNORED_COMPARISON_FIELDS:
271-
if (
272-
k not in self._data
273-
or k not in other._data
274-
or self._data[k] != other._data[k]
275-
):
276-
_LOGGER.info(
277-
"Backup %s and %s not equal because %s field has different value: %s and %s",
278-
self.slug,
279-
other.slug,
280-
k,
281-
self._data.get(k),
282-
other._data.get(k),
283-
)
284-
return False
285-
return True
267+
def __hash__(self) -> int:
268+
"""Return hash of backup."""
269+
return hash(self.slug)
286270

287271
def consolidate(self, backup: Self) -> None:
288272
"""Consolidate two backups with same slug in different locations."""
289-
if self.slug != backup.slug:
273+
if self != backup:
290274
raise ValueError(
291275
f"Backup {self.slug} and {backup.slug} are not the same backup"
292276
)
293-
if self != backup:
294-
raise BackupInvalidError(
295-
f"Backup in {backup.location} and {self.location} both have slug {self.slug} but are not the same!"
296-
)
297277

298-
# In case of conflict we always ignore the ones from the first one. But log them to let the user know
278+
# Compare all fields except ones about protection. Current encryption status does not affect equality
279+
other_data = backup._data # pylint: disable=protected-access
280+
keys = self._data.keys() | other_data.keys()
281+
for k in keys - IGNORED_COMPARISON_FIELDS:
282+
if (
283+
k not in self._data
284+
or k not in other_data
285+
or self._data[k] != other_data[k]
286+
):
287+
raise BackupInvalidError(
288+
f"Cannot consolidate backups in {backup.location} and {self.location} with slug {self.slug} "
289+
f"because field {k} has different values: {self._data.get(k)} and {other_data.get(k)}!",
290+
_LOGGER.error,
291+
)
299292

293+
# In case of conflict we always ignore the ones from the first one. But log them to let the user know
300294
if conflict := {
301295
loc: val.path
302296
for loc, val in self.all_locations.items()

supervisor/mounts/mount.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,14 @@ async def is_mounted(self) -> bool:
164164
"""Return true if successfully mounted and available."""
165165
return self.state == UnitActiveState.ACTIVE
166166

167-
def __eq__(self, other):
167+
def __eq__(self, other: object) -> bool:
168168
"""Return true if mounts are the same."""
169169
return isinstance(other, Mount) and self.name == other.name
170170

171+
def __hash__(self) -> int:
172+
"""Return hash of mount."""
173+
return hash(self.name)
174+
171175
async def load(self) -> None:
172176
"""Initialize object."""
173177
# If there's no mount unit, mount it to make one

tests/backups/test_backup.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,33 @@ async def test_consolidate(
185185
}
186186

187187

188+
@pytest.mark.usefixtures("tmp_supervisor_data")
189+
async def test_consolidate_failure(coresys: CoreSys, tmp_path: Path):
190+
"""Test consolidate with two backups that are not the same."""
191+
(mount_dir := coresys.config.path_mounts / "backup_test").mkdir()
192+
tar1 = Path(copy(get_fixture_path("test_consolidate_unc.tar"), tmp_path))
193+
backup1 = Backup(coresys, tar1, "test", None)
194+
await backup1.load()
195+
196+
tar2 = Path(copy(get_fixture_path("backup_example.tar"), mount_dir))
197+
backup2 = Backup(coresys, tar2, "test", "backup_test")
198+
await backup2.load()
199+
200+
with pytest.raises(
201+
ValueError,
202+
match=f"Backup {backup1.slug} and {backup2.slug} are not the same backup",
203+
):
204+
backup1.consolidate(backup2)
205+
206+
# Force slugs to be the same to run the fields check
207+
backup1._data["slug"] = backup2.slug # pylint: disable=protected-access
208+
with pytest.raises(
209+
BackupInvalidError,
210+
match=f"Cannot consolidate backups in {backup2.location} and {backup1.location} with slug {backup1.slug}",
211+
):
212+
backup1.consolidate(backup2)
213+
214+
188215
@pytest.mark.parametrize(
189216
(
190217
"tarfile_side_effect",

0 commit comments

Comments
 (0)