diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py index f575ac0..66a4eac 100644 --- a/selfprivacy_api/backup/__init__.py +++ b/selfprivacy_api/backup/__init__.py @@ -395,11 +395,8 @@ class Backups: auto_snaps = Backups._auto_snaps(service) new_snaplist = Backups._prune_snaps_with_quotas(auto_snaps) - # TODO: Can be optimized since there is forgetting of an array in one restic op - # but most of the time this will be only one snap to forget. - for snap in auto_snaps: - if snap not in new_snaplist: - Backups.forget_snapshot(snap) + deletable_snaps = [snap for snap in auto_snaps if snap not in new_snaplist] + Backups.forget_snapshots(deletable_snaps) @staticmethod def _standardize_quotas(i: int) -> int: @@ -426,7 +423,10 @@ class Backups: yearly=Backups._standardize_quotas(quotas.yearly), # type: ignore ) ) + # do not prune all autosnaps right away, this will be done by an async task + @staticmethod + def prune_all_autosnaps() -> None: for service in get_all_services(): Backups._prune_auto_snaps(service) @@ -606,6 +606,19 @@ class Backups: return snap + @staticmethod + def forget_snapshots(snapshots: List[Snapshot]) -> None: + """ + Deletes a batch of snapshots from the repo and from cache + Optimized + """ + ids = [snapshot.id for snapshot in snapshots] + Backups.provider().backupper.forget_snapshots(ids) + + # less critical + for snapshot in snapshots: + Storage.delete_cached_snapshot(snapshot) + @staticmethod def forget_snapshot(snapshot: Snapshot) -> None: """Deletes a snapshot from the repo and from cache""" @@ -614,11 +627,11 @@ class Backups: @staticmethod def forget_all_snapshots(): - """deliberately erase all snapshots we made""" - # there is no dedicated optimized command for this, - # but maybe we can have a multi-erase - for snapshot in Backups.get_all_snapshots(): - Backups.forget_snapshot(snapshot) + """ + Mark all snapshots we have made for deletion and make them inaccessible + (this is done by cloud, we only issue a command) + """ + Backups.forget_snapshots(Backups.get_all_snapshots()) @staticmethod def force_snapshot_cache_reload() -> None: diff --git a/selfprivacy_api/backup/backuppers/__init__.py b/selfprivacy_api/backup/backuppers/__init__.py index 0067a41..46a719e 100644 --- a/selfprivacy_api/backup/backuppers/__init__.py +++ b/selfprivacy_api/backup/backuppers/__init__.py @@ -66,3 +66,8 @@ class AbstractBackupper(ABC): def forget_snapshot(self, snapshot_id) -> None: """Forget a snapshot""" raise NotImplementedError + + @abstractmethod + def forget_snapshots(self, snapshot_ids: List[str]) -> None: + """Maybe optimized deletion of a batch of snapshots, just cycling if unsupported""" + raise NotImplementedError diff --git a/selfprivacy_api/backup/backuppers/none_backupper.py b/selfprivacy_api/backup/backuppers/none_backupper.py index 429d9ab..86e25a6 100644 --- a/selfprivacy_api/backup/backuppers/none_backupper.py +++ b/selfprivacy_api/backup/backuppers/none_backupper.py @@ -39,4 +39,7 @@ class NoneBackupper(AbstractBackupper): raise NotImplementedError def forget_snapshot(self, snapshot_id): - raise NotImplementedError + raise NotImplementedError("forget_snapshot") + + def forget_snapshots(self, snapshots): + raise NotImplementedError("forget_snapshots") diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index afa6295..fd653e6 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -86,6 +86,10 @@ class ResticBackupper(AbstractBackupper): return f"echo {LocalBackupSecret.get()}" def restic_command(self, *args, tags: Optional[List[str]] = None) -> List[str]: + """ + Construct a restic command against the currently configured repo + Can support [nested] arrays as arguments, will flatten them into the final commmand + """ if tags is None: tags = [] @@ -384,15 +388,15 @@ class ResticBackupper(AbstractBackupper): output, ) + def forget_snapshot(self, snapshot_id: str) -> None: + self.forget_snapshots([snapshot_id]) + @unlocked_repo - def forget_snapshot(self, snapshot_id) -> None: - """ - Either removes snapshot or marks it for deletion later, - depending on server settings - """ + def forget_snapshots(self, snapshot_ids: List[str]) -> None: + # in case the backupper program supports batching, otherwise implement it by cycling forget_command = self.restic_command( "forget", - snapshot_id, + [snapshot_ids], # TODO: prune should be done in a separate process "--prune", ) @@ -414,7 +418,7 @@ class ResticBackupper(AbstractBackupper): if "no matching ID found" in err: raise ValueError( - "trying to delete, but no such snapshot: ", snapshot_id + "trying to delete, but no such snapshot(s): ", snapshot_ids ) assert ( diff --git a/selfprivacy_api/backup/tasks.py b/selfprivacy_api/backup/tasks.py index bdf6c9f..f0422ca 100644 --- a/selfprivacy_api/backup/tasks.py +++ b/selfprivacy_api/backup/tasks.py @@ -3,13 +3,18 @@ The tasks module contains the worker tasks that are used to back up and restore """ from datetime import datetime, timezone -from selfprivacy_api.graphql.common_types.backup import RestoreStrategy, BackupReason +from selfprivacy_api.graphql.common_types.backup import ( + RestoreStrategy, + BackupReason, +) from selfprivacy_api.models.backup.snapshot import Snapshot from selfprivacy_api.utils.huey import huey from huey import crontab from selfprivacy_api.services.service import Service from selfprivacy_api.backup import Backups +from selfprivacy_api.jobs import Jobs, JobStatus, Job + SNAPSHOT_CACHE_TTL_HOURS = 6 @@ -36,6 +41,22 @@ def start_backup( return True +@huey.task() +def prune_autobackup_snapshots(job: Job) -> bool: + """ + Remove all autobackup snapshots that do not fit into quotas set + """ + Jobs.update(job, JobStatus.RUNNING) + try: + Backups.prune_all_autosnaps() + except Exception as e: + Jobs.update(job, JobStatus.ERROR, error=type(e).__name__ + ":" + str(e)) + return False + + Jobs.update(job, JobStatus.FINISHED) + return True + + @huey.task() def restore_snapshot( snapshot: Snapshot, diff --git a/selfprivacy_api/graphql/mutations/backup_mutations.py b/selfprivacy_api/graphql/mutations/backup_mutations.py index dcfebff..cc1538e 100644 --- a/selfprivacy_api/graphql/mutations/backup_mutations.py +++ b/selfprivacy_api/graphql/mutations/backup_mutations.py @@ -1,6 +1,8 @@ import typing import strawberry +from selfprivacy_api.jobs import Jobs + from selfprivacy_api.graphql import IsAuthenticated from selfprivacy_api.graphql.mutations.mutation_interface import ( GenericMutationReturn, @@ -18,7 +20,11 @@ from selfprivacy_api.graphql.common_types.backup import ( from selfprivacy_api.backup import Backups from selfprivacy_api.services import get_service_by_id -from selfprivacy_api.backup.tasks import start_backup, restore_snapshot +from selfprivacy_api.backup.tasks import ( + start_backup, + restore_snapshot, + prune_autobackup_snapshots, +) from selfprivacy_api.backup.jobs import add_backup_job, add_restore_job @@ -103,8 +109,16 @@ class BackupMutations: To disable autobackup use autobackup period setting, not this mutation. """ + job = Jobs.add( + name="Trimming autobackup snapshots", + type_id="backups.autobackup_trimming", + description="Pruning the excessive snapshots after the new autobackup quotas are set", + ) + try: Backups.set_autobackup_quotas(quotas) + # this task is async and can fail with only a job to report the error + prune_autobackup_snapshots(job) return GenericBackupConfigReturn( success=True, message="", @@ -115,7 +129,7 @@ class BackupMutations: except Exception as e: return GenericBackupConfigReturn( success=False, - message=str(e), + message=type(e).__name__ + ":" + str(e), code=400, configuration=Backup().configuration(), ) diff --git a/tests/test_graphql/test_api_backup.py b/tests/test_graphql/test_api_backup.py index 225abf7..b44fd44 100644 --- a/tests/test_graphql/test_api_backup.py +++ b/tests/test_graphql/test_api_backup.py @@ -265,6 +265,10 @@ def api_init_without_key( def assert_ok(data): + if data["success"] is False: + # convenience for debugging, this should display error + # if empty, consider adding helpful messages + raise ValueError(data["code"], data["message"]) assert data["code"] == 200 assert data["success"] is True @@ -302,7 +306,7 @@ def test_snapshots_empty(authorized_client, dummy_service): assert snaps == [] -def test_start_backup(authorized_client, dummy_service): +def test_start_backup(authorized_client, dummy_service, backups): response = api_backup(authorized_client, dummy_service) data = get_data(response)["backup"]["startBackup"] assert data["success"] is True @@ -318,7 +322,7 @@ def test_start_backup(authorized_client, dummy_service): assert snap["service"]["id"] == "testservice" -def test_restore(authorized_client, dummy_service): +def test_restore(authorized_client, dummy_service, backups): api_backup(authorized_client, dummy_service) snap = api_snapshots(authorized_client)[0] assert snap["id"] is not None @@ -331,7 +335,7 @@ def test_restore(authorized_client, dummy_service): assert Jobs.get_job(job["uid"]).status == JobStatus.FINISHED -def test_reinit(authorized_client, dummy_service, tmpdir): +def test_reinit(authorized_client, dummy_service, tmpdir, backups): test_repo_path = path.join(tmpdir, "not_at_all_sus") response = api_init_without_key( authorized_client, "FILE", "", "", test_repo_path, "" @@ -353,7 +357,7 @@ def test_reinit(authorized_client, dummy_service, tmpdir): assert Jobs.get_job(job["uid"]).status == JobStatus.FINISHED -def test_remove(authorized_client, generic_userdata): +def test_remove(authorized_client, generic_userdata, backups): response = api_remove(authorized_client) data = get_data(response)["backup"]["removeRepository"] assert_ok(data) @@ -367,7 +371,7 @@ def test_remove(authorized_client, generic_userdata): assert configuration["isInitialized"] is False -def test_autobackup_quotas_nonzero(authorized_client): +def test_autobackup_quotas_nonzero(authorized_client, backups): quotas = _AutobackupQuotas( last=3, daily=2, @@ -383,7 +387,7 @@ def test_autobackup_quotas_nonzero(authorized_client): assert configuration["autobackupQuotas"] == quotas -def test_autobackup_period_nonzero(authorized_client): +def test_autobackup_period_nonzero(authorized_client, backups): new_period = 11 response = api_set_period(authorized_client, new_period) data = get_data(response)["backup"]["setAutobackupPeriod"] @@ -393,7 +397,7 @@ def test_autobackup_period_nonzero(authorized_client): assert configuration["autobackupPeriod"] == new_period -def test_autobackup_period_zero(authorized_client): +def test_autobackup_period_zero(authorized_client, backups): new_period = 0 # since it is none by default, we better first set it to something non-negative response = api_set_period(authorized_client, 11) @@ -406,7 +410,7 @@ def test_autobackup_period_zero(authorized_client): assert configuration["autobackupPeriod"] == None -def test_autobackup_period_none(authorized_client): +def test_autobackup_period_none(authorized_client, backups): # since it is none by default, we better first set it to something non-negative response = api_set_period(authorized_client, 11) # and now we nullify it @@ -418,7 +422,7 @@ def test_autobackup_period_none(authorized_client): assert configuration["autobackupPeriod"] == None -def test_autobackup_period_negative(authorized_client): +def test_autobackup_period_negative(authorized_client, backups): # since it is none by default, we better first set it to something non-negative response = api_set_period(authorized_client, 11) # and now we nullify it @@ -432,7 +436,7 @@ def test_autobackup_period_negative(authorized_client): # We cannot really check the effect at this level, we leave it to backend tests # But we still make it run in both empty and full scenarios and ask for snaps afterwards -def test_reload_snapshots_bare_bare_bare(authorized_client, dummy_service): +def test_reload_snapshots_bare_bare_bare(authorized_client, dummy_service, backups): api_remove(authorized_client) response = api_reload_snapshots(authorized_client) @@ -443,7 +447,7 @@ def test_reload_snapshots_bare_bare_bare(authorized_client, dummy_service): assert snaps == [] -def test_reload_snapshots(authorized_client, dummy_service): +def test_reload_snapshots(authorized_client, dummy_service, backups): response = api_backup(authorized_client, dummy_service) data = get_data(response)["backup"]["startBackup"] @@ -455,7 +459,7 @@ def test_reload_snapshots(authorized_client, dummy_service): assert len(snaps) == 1 -def test_forget_snapshot(authorized_client, dummy_service): +def test_forget_snapshot(authorized_client, dummy_service, backups): response = api_backup(authorized_client, dummy_service) data = get_data(response)["backup"]["startBackup"] @@ -470,7 +474,7 @@ def test_forget_snapshot(authorized_client, dummy_service): assert len(snaps) == 0 -def test_forget_nonexistent_snapshot(authorized_client, dummy_service): +def test_forget_nonexistent_snapshot(authorized_client, dummy_service, backups): snaps = api_snapshots(authorized_client) assert len(snaps) == 0 response = api_forget(authorized_client, "898798uekiodpjoiweoiwuoeirueor") diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 27a2879..b66a90d 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -1,52 +1,49 @@ import pytest + import os import os.path as path from os import makedirs from os import remove from os import listdir from os import urandom -from datetime import datetime, timedelta, timezone, date, time -from subprocess import Popen + +from datetime import datetime, timedelta, timezone from copy import copy - -import secrets - - import tempfile from selfprivacy_api.utils.huey import huey import selfprivacy_api.services as services from selfprivacy_api.services import Service, get_all_services -from selfprivacy_api.services.service import ServiceStatus - from selfprivacy_api.services import get_service_by_id +from selfprivacy_api.services.service import ServiceStatus from selfprivacy_api.services.test_service import DummyService + from selfprivacy_api.graphql.queries.providers import BackupProvider -from selfprivacy_api.graphql.common_types.backup import RestoreStrategy, BackupReason +from selfprivacy_api.graphql.common_types.backup import ( + RestoreStrategy, + BackupReason, + AutobackupQuotas, +) + from selfprivacy_api.jobs import Jobs, JobStatus from selfprivacy_api.models.backup.snapshot import Snapshot -from selfprivacy_api.graphql.common_types.backup import AutobackupQuotas - from selfprivacy_api.backup import Backups, BACKUP_PROVIDER_ENVS import selfprivacy_api.backup.providers as providers from selfprivacy_api.backup.providers import AbstractBackupProvider from selfprivacy_api.backup.providers.backblaze import Backblaze from selfprivacy_api.backup.providers.none import NoBackups from selfprivacy_api.backup.util import sync -from selfprivacy_api.backup.backuppers.restic_backupper import ResticBackupper -from selfprivacy_api.backup.jobs import add_backup_job, add_restore_job - from selfprivacy_api.backup.tasks import ( start_backup, restore_snapshot, reload_snapshot_cache, + prune_autobackup_snapshots, ) from selfprivacy_api.backup.storage import Storage -from selfprivacy_api.backup.jobs import get_backup_job TESTFILE_BODY = "testytest!" @@ -651,6 +648,9 @@ def test_too_many_auto(backups, dummy_service): # Retroactivity quota.last = 1 Backups.set_autobackup_quotas(quota) + job = Jobs.add("trimming", "test.autobackup_trimming", "trimming the snaps!") + handle = prune_autobackup_snapshots(job) + handle(blocking=True) snaps = Backups.get_snapshots(dummy_service) assert len(snaps) == 1