Skip to content

Add backup export/import to the admin panel setup page - #899

Open
sven-n wants to merge 1 commit into
masterfrom
claude/pr-715-review-fixes-t13zog
Open

Add backup export/import to the admin panel setup page#899
sven-n wants to merge 1 commit into
masterfrom
claude/pr-715-review-fixes-t13zog

Conversation

@sven-n

@sven-n sven-n commented Aug 25, 2026

Copy link
Copy Markdown
Member

Takes over the work of #715, rebased onto the current master, with the open issues fixed. Fixes #462.

The original branch (copilot/add-backup-restore-functionality) is based on v0.9.9 and conflicts with master, so the change is applied onto the current master here instead of merging that branch.

Taken over from #715

  • IBackupService/BackupService in the MUnique.OpenMU.Persistence project, without any dependency to the entity framework - it works with the repositories which are available through IPersistenceContextProvider/IContext.
  • InMemoryBackupService, which supports the export and not the restore.
  • BackupController for the download and the export/import UI on the setup page, including the progress spinner during the import.
  • LocalizedStringJsonConverter in the serializer/deserializer options and the BrowserFileStream -> MemoryStream copy before the import, which fixed the runtime errors reported in Add Export/Import backup functionality to the admin panel Setup page #715.

What was still broken, and is fixed here

The restore didn't restore anything but the root objects. The exported json writes collections in the reference-preserving form ({ "$id": ..., "$values": [ ... ] }), but ReferenceResolvingConverter only accepted plain arrays - the shape the postgres json query produces - and silently skipped everything else. A restored GameConfiguration therefore had 0 maps, 0 items, 0 character classes, 0 attributes and 0 monsters. It now reads both forms; the existing postgres path is unaffected.

Mapster now ignores properties marked with TransientAttribute, instead of scanning the member types for IElement as the previous commit did. That's what those properties (e.g. of the SkillEntry) actually are, and it also covers the transient IAttributeSystem one, which the type scan missed. The generator template and the generated file were updated together, and re-running the generator produces exactly the checked-in file. The Mapster.CompileException reported in #715 is reproducible with the rule removed and gone with it in place.

The object copy of the restore doesn't swallow exceptions anymore. A failing restore is now visible instead of resulting in silently missing data. Two failures which had been hidden by the empty catch blocks are fixed: indexer properties caused a TargetParameterCountException (they are skipped now) and the Add-method is taken from the ICollection<T> interface, because the implementing type may have other Add overloads.

The import checks the file before the database is re-created. ContainsRestorableData verifies that the uploaded file is a zip archive with known backup entries, so that selecting a wrong file doesn't drop the existing data.

Removed the unused upload action of the BackupController. It re-created the database before it knew whether the upload was a backup at all, and it skipped the DatabaseInitialized event of the SetupService. The import on the setup page is the only path now.

Additionally: the Setup.razor conflict with master, some StyleCop warnings (SA1202, SA1204, SA1208) and missing newlines at the end of files.

Test

BackupServiceTests does a round trip with the in-memory persistence: create the season six initialization data, export it, restore it into a second context provider and compare. It asserts the object counts per type, the collection counts of the configuration, the account/character data, and that references between the different backup files point to the same restored instances. A second test covers that a file which is no backup archive is detected as such.

The full solution builds and the whole test suite passes locally (-p:ci=true).

Not covered

The restore was verified against the in-memory persistence only, since this environment has no postgres. The export was already confirmed to work with the entity framework in #715, and the mapping fix was verified separately against the EF model types, but a restore against a real database should be tried before merging.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LhfyPthjrRZ6K9CgVd26RT


Generated by Claude Code

This takes over the work of PR #715 (copilot/add-backup-restore-functionality),
rebased onto the current master, and fixes the issues which were left open.

Taken over from the pull request:
* IBackupService/BackupService in the Persistence project, without any
  dependency to the entity framework - it works with the repositories which
  are available through IPersistenceContextProvider/IContext.
* InMemoryBackupService which only supports the export.
* BackupController and the export/import UI on the setup page.

Fixed on top of it:
* The restore didn't restore anything but the root objects: the exported json
  writes collections in the reference-preserving form ({ "$id", "$values" }),
  which the ReferenceResolvingConverter silently skipped, because it only
  accepted plain arrays. It now reads both forms.
* Mapster ignores properties which are marked with the TransientAttribute,
  instead of scanning the member types for IElement. That's what the transient
  properties (e.g. of the SkillEntry) actually are, and it also covers the
  IAttributeSystem ones. Verified that the previously reported Mapster
  CompileException is reproducible without the rule and gone with it.
* The object copy of the restore doesn't swallow exceptions anymore, so a
  failing restore is visible instead of resulting in missing data. Indexer
  properties are skipped (they caused a TargetParameterCountException) and the
  Add-method is taken from the ICollection<T> interface.
* The import checks the uploaded file before the database is re-created, so
  that selecting a wrong file doesn't drop the data.
* Removed the unused upload action of the BackupController - it re-created the
  database before it knew if the upload was a backup at all, and skipped the
  DatabaseInitialized event of the SetupService.
* Fixed the merge conflict with the current master, some StyleCop warnings and
  missing newlines at the end of files.

Added a round trip test (export, restore into another context provider,
compare) which covers the data of the season six data initialization,
including that references between the backup files point to the same restored
instances.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01LhfyPthjrRZ6K9CgVd26RT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backup/Restore over the admin panel

2 participants