Repository navigation
Include uploaded images in user backup and restore - #3474
Closed
AlanFarley wants to merge 1 commit into
Closed
AlanFarley wants to merge 1 commit into
AlanFarley wants to merge 1 commit into
Conversation
User ZIP backups currently contain stations, logbooks and QSOs but omit their uploaded images. This change includes QSL, eQSL, SSTV and diary images, and restores their associations after record IDs change. It also preserves station-to-logbook relationships. The new schema (1.1) records attachment paths and SHA-256 checksums. Imports validate image formats, checksums and size limits, reuse existing records and attachments, and use a database transaction with cleanup of newly created files on failure. Diary entries with images can be restored optionally; newly created diary entries are private. Older backups without attachments remain supported. The preview displays attachment counts and missing-source-file notices; the result displays the number of restored attachments. No database migration or update-script changes are required. Validation: - php tests/backup_files_test.php: passed (temporary database tables and temporary files). - PHP syntax checks: passed for the controller, model, three views and test. - git diff --check: passed. - Cypress and browser JavaScript validation: pending; Cypress is not installed locally and existing specs do not cover backup. Limitations: - Only diary entries with available image attachments are included. - Images increase archive size and restore duration; server upload limits still apply. - Import limits: 256 MiB JSON in ZIP, 100 MiB per image, 2 GiB total image data, 100,000 attachment references. - Review and browser validation are still required before publication.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved security, rollback, and attachment-restoration defects prevent safe approval.
Review effort: Balanced
Findings: 2
Open (10)
What changed in this PR
Extends Cloudlog’s user backups to include uploaded images and preserve their associations when restoring records.
Changes:
- Adds schema 1.1 attachment manifests, checksums, and archive validation.
- Restores image associations, station–logbook relationships, and optional private diary entries.
- Updates backup previews/results and adds regression tests.
| File | Description |
|---|---|
| tests/backup_files_test.php | Tests export, validation, restore, and failure handling. |
| application/views/backup/main.php | Describes image backups and displays notices. |
| application/views/backup/import_progress.php | Displays restored attachment counts. |
| application/views/backup/import_preview.php | Shows image counts, warnings, and diary selection. |
| application/models/Backup_files.php | Implements attachment export, validation, restore, and cleanup. |
| application/controllers/Backup.php | Coordinates archive staging, ID mapping, relationships, and transactions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+303
to
+304
| if (!$this->db->trans_begin()) throw new RuntimeException('Could not start the restore transaction.'); | ||
| $transaction_started = true; |
| $diary_map = array(); | ||
| if ($include_diary) { | ||
| foreach (isset($manifest['diary_entries']) ? $manifest['diary_entries'] : array() as $note) { | ||
| $old_id = $note['id']; |
| $this->db->insert('station_logbooks', $logbook_copy); $logbook_id_map[$logbook['logbook_id']] = $this->db->insert_id(); $imported['logbooks']++; | ||
| } | ||
| foreach (isset($data['logbook_relationships']) ? $data['logbook_relationships'] : array() as $relation) { | ||
| if (!isset($logbook_id_map[$relation['station_logbook_id']], $station_id_map[$relation['station_location_id']])) continue; |
| foreach (array_chunk($logbook['qsos'], 1000) as $qso_chunk) { | ||
| foreach ($qso_chunk as $qso) { | ||
| unset($qso['COL_PRIMARY_KEY']); if (isset($qso['station_id']) && isset($station_id_map[$qso['station_id']])) { $qso['station_id'] = $station_id_map[$qso['station_id']]; } | ||
| if (!isset($qso['station_id'], $station_id_map[$qso['station_id']])) continue; |
Comment on lines
+376
to
+378
| } catch (Exception $e) { | ||
| if ($transaction_started) $this->db->trans_rollback(); | ||
| $this->Backup_files->rollback_files(); |
Comment on lines
+43
to
+44
| $manifest['files'][] = array('type' => $type, 'owner_id' => $row[$settings['id']], 'entry' => $entry, 'sha256' => $hash, | ||
| 'caption' => isset($row['caption']) ? $row['caption'] : null, 'sort_order' => isset($row['sort_order']) ? (int)$row['sort_order'] : 0); |
Comment on lines
+95
to
+102
| !preg_match('#^files/'.preg_quote($file['type'], '#').'/'.$file['sha256'].'\.(jpg|png|gif|webp)$#D', $file['entry'])) { | ||
| throw new RuntimeException('Invalid backup image reference.'); | ||
| } | ||
| } | ||
|
|
||
| private function image_extension($path) { | ||
| $info = @getimagesize($path); | ||
| $extensions = array(IMAGETYPE_JPEG => 'jpg', IMAGETYPE_PNG => 'png', IMAGETYPE_GIF => 'gif', IMAGETYPE_WEBP => 'webp'); |
| foreach (isset($manifest['diary_entries']) ? $manifest['diary_entries'] : array() as $note) { | ||
| $old_id = $note['id']; | ||
| // Reuse an existing entry belonging to this user on repeated imports. | ||
| $existing = $this->db->where('user_id', $user_id)->where('title', $note['title'])->where('cat', $note['cat'])->where('created_at', $note['created_at'])->where('note', $note['note'])->get('notes')->row_array(); |
| $settings = $this->types[$file['type']]; | ||
| $map = $file['type'] === 'diary' ? $diary_map : $qso_map; | ||
| if (empty($map[$file['owner_id']])) continue; | ||
| $directory = FCPATH.$settings['directory'].($file['type'] === 'diary' ? $user_id.'/' : ''); |
| if (empty($map[$file['owner_id']])) continue; | ||
| $directory = FCPATH.$settings['directory'].($file['type'] === 'diary' ? $user_id.'/' : ''); | ||
| if (!is_dir($directory) && !mkdir($directory, 0755, true)) throw new RuntimeException('Could not create image directory.'); | ||
| $filename = 'backup_'.$file['sha256'].'.'.pathinfo($file['entry'], PATHINFO_EXTENSION); |
1 of 5 tasks
Owner
|
Thanks for this. Copilot's review found a few restore problems that would be unsafe to merge as-is, so I have put the fixes on #3475 (branch What changed:
|
Owner
|
Closing this in favour of #3475, which has the same change plus the review fixes. Please test that branch; your original commit is included there. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User ZIP backups currently contain stations, logbooks and QSOs but omit their uploaded images. This change includes QSL, eQSL, SSTV and diary images, and restores their associations after record IDs change. It also preserves station-to-logbook relationships.
The new schema (1.1) records attachment paths and SHA-256 checksums. Imports validate image formats, checksums and size limits, reuse existing records and attachments, and use a database transaction with cleanup of newly created files on failure. Diary entries with images can be restored optionally; newly created diary entries are private. Older backups without attachments remain supported.
The preview displays attachment counts and missing-source-file notices; the result displays the number of restored attachments. No database migration or update-script changes are required.
Validation:
Limitations: