feat: prune remote backups on retention + consolidate backup helpers (#229, #230) #231

2026-07-06T01:34:35Z

jmiller commented

2026-07-06 01:34:35 +00:00

Addresses two related asks: retention must apply to remote destinations, and all backup functions should live in shared helpers.

#229 — Retention now prunes remote copies

Previously retention deleted only the local record + archive + .log; remote copies (SFTP/FTP/S3/Google Drive) grew unbounded.

Added delete(string $remoteName): array to RemoteUploaderInterface and implemented it in all four uploaders. Deletes are idempotent (an already-absent remote file counts as success).
RetentionManager now removes each pruned archive from the profile's currently-enabled remotes — best-effort, failures logged, never blocking local pruning.
restore.php is intentionally NOT deleted — it's a single fixed-name file every backup overwrites, so newer backups still depend on it. (Deleting it during old-archive pruning would break restore for the copies that remain.)
Known limitation (documented): records don't store their destinations, so deletion targets the profile's current enabled remotes; a destination removed after a backup won't be reconciled.

#230 — Shared-helper consolidation

RemoteUploaderFactory (new) — single home for the type→class map + key-prefixing that was duplicated verbatim in BackupEngine and SteppedBackupEngine; both now delegate to it (and retention uses it for deletion).
RetentionManager is the single retention authority — takes the global max_age_days/max_backups fallback and gained pruneOrphans(). The system plugin's hourly doCleanup() delegates to it; its duplicate deleteBackupRecord() is removed. (Bonus: the plugin sweep now also prunes warning records, not just complete.)
Backend controller, API controller, and legacy cli/mokosuitebackup.php now run through the shared BackupRunner (gaining the normalized complete/warning/fail status) instead of new BackupEngine().

The dashboard AJAX SteppedBackupEngine interactive path is intentionally left separate.

Verify

php -l clean on all 13 changed files.
RetentionManager::prune() signature change is backward-compatible (new params default to 0); the two engine callers still pass 2 args.
No remaining new BackupEngine() outside BackupRunner; deleteBackupRecord fully removed.

Refs #229 #230

https://claude.ai/code/session_01WbGBN9VyRK61zczYWcCQ2i

Addresses two related asks: retention must apply to remote destinations, and all backup functions should live in shared helpers. ## #229 — Retention now prunes remote copies Previously retention deleted only the **local** record + archive + `.log`; remote copies (SFTP/FTP/S3/Google Drive) grew unbounded. - Added `delete(string $remoteName): array` to `RemoteUploaderInterface` and implemented it in all four uploaders. Deletes are **idempotent** (an already-absent remote file counts as success). - `RetentionManager` now removes each pruned archive from the profile's **currently-enabled** remotes — best-effort, failures logged, never blocking local pruning. - **`restore.php` is intentionally NOT deleted** — it's a single fixed-name file every backup overwrites, so newer backups still depend on it. (Deleting it during old-archive pruning would break restore for the copies that remain.) - Known limitation (documented): records don't store their destinations, so deletion targets the profile's *current* enabled remotes; a destination removed after a backup won't be reconciled. ## #230 — Shared-helper consolidation - **`RemoteUploaderFactory`** (new) — single home for the type→class map + key-prefixing that was duplicated verbatim in `BackupEngine` and `SteppedBackupEngine`; both now delegate to it (and retention uses it for deletion). - **`RetentionManager` is the single retention authority** — takes the global `max_age_days`/`max_backups` fallback and gained `pruneOrphans()`. The system plugin's hourly `doCleanup()` delegates to it; its duplicate `deleteBackupRecord()` is removed. (Bonus: the plugin sweep now also prunes `warning` records, not just `complete`.) - **Backend controller, API controller, and legacy `cli/mokosuitebackup.php`** now run through the shared `BackupRunner` (gaining the normalized complete/warning/fail status) instead of `new BackupEngine()`. The dashboard AJAX `SteppedBackupEngine` interactive path is intentionally left separate. ## Verify - `php -l` clean on all 13 changed files. - `RetentionManager::prune()` signature change is backward-compatible (new params default to 0); the two engine callers still pass 2 args. - No remaining `new BackupEngine()` outside `BackupRunner`; `deleteBackupRecord` fully removed. Refs #229 #230 https://claude.ai/code/session_01WbGBN9VyRK61zczYWcCQ2i

jmiller added 1 commit 2026-07-06 01:34:36 +00:00

feat: prune remote backups on retention + consolidate backup helpers

Universal: PR Check / Require Docs Update (pull_request) Has been skipped

Details

Universal: PR Check / Wiki Update Reminder (pull_request) Has been skipped

Details

Universal: PR Check / Branch Policy (pull_request) Successful in 2s

Details

Universal: PR Check / Secret Scan (pull_request) Successful in 10s

Details

Universal: PR Check / Validate PR (pull_request) Failing after 14s

Details

Universal: Pre-Release / Build Pre-Release (${{ inputs.stability || github.ref_name }}) (push) Successful in 40s

Details

Generic: Project CI / Lint & Validate (pull_request) Successful in 52s

Details

Joomla: Metadata Validation / Validate Joomla Metadata (pull_request) Successful in 1m8s

Details

Generic: Project CI / Tests (pull_request) Has been cancelled

Details

Universal: PR Check / Build RC Package (pull_request) Has been cancelled

Details

Universal: PR Check / Report Issues (pull_request) Has been cancelled

Details

447f7b572e

Retention (RetentionManager and the plugin's hourly cleanup) only ever
deleted local files; remote archives grew unbounded. Add an idempotent
delete() to RemoteUploaderInterface + all four uploaders and have
RetentionManager remove each pruned archive from the profile's enabled
remotes (best-effort; failures logged). The shared restore.php is left
in place since every backup overwrites it. (#229)

Consolidate duplicated plumbing (#230):
- New RemoteUploaderFactory replaces the createUploaderFromParams copy
  duplicated in BackupEngine and SteppedBackupEngine.
- RetentionManager becomes the single retention authority (global-default
  fallback + pruneOrphans()); the system plugin delegates to it and its
  duplicate doCleanup()/deleteBackupRecord() logic is removed.
- Backend controller, API controller and legacy cli/mokosuitebackup.php
  now run through the shared BackupRunner instead of BackupEngine directly.

Refs #229 #230

Claude-Session: https://claude.ai/code/session_01WbGBN9VyRK61zczYWcCQ2i

jmiller added 1 commit 2026-07-06 01:34:53 +00:00

chore(version): pre-release bump to 02.58.06-dev [skip ci]

RC Revert / Rename rc/ back to dev/ (pull_request) Has been skipped

Details

Branch Cleanup / Delete merged branch (pull_request) Successful in 2s

Details

7a829b6eca

jmiller merged commit b729aa5052 into dev

2026-07-06 01:35:45 +00:00

jmiller deleted branch fix/retention-remote-shared-helper

2026-07-06 01:35:46 +00:00

jmiller referenced this pull request