Add backup export/import to the admin panel setup page - #899
Open
sven-n wants to merge 3 commits into
Open
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LhfyPthjrRZ6K9CgVd26RT
…w-fixes-t13zog # Conflicts: # src/Web/AdminPanel/Properties/Resources.Designer.cs
Deploying openmudocs with
|
| Latest commit: |
ceaf99f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dee93e60.openmudocs.pages.dev |
| Branch Preview URL: | https://claude-pr-715-review-fixes-t.openmudocs.pages.dev |
Verified the backup against a real postgres database: create the season six data, export it, re-create the database, restore the backup and compare every row of every table with the source. That found the reported circular dependency and several silent data losses. Now 34710 of 34786 rows are restored with identical content - the remaining 76 rows are objects which nothing in the source database references (leftovers of the data initialization), so they are not reachable for the export. * Circular dependency on save: the root object is now created and saved before it's filled with its data, like the data initialization does it for the game configuration. The entity framework can't insert GameConfiguration -> GameMapDefinition -> CastleSiegeConfiguration -> GameConfiguration in one go. * Many-to-many relations (character classes of an item, drop item groups of a map, skills of a master skill definition, ...) were lost, because the entity framework model has no "Raw" collection for them. The items are added to the collection of the data model now, which creates the join entities. * Collections of value types (ItemSlotType.ItemSlots) were lost twice: Mapster created a new, empty collection instead of filling the existing one, and the deserializer ignored the property, because it has no public setter. * Properties which can only be set by a constructor (ConstValueAttribute.Value - 394 rows of character class base values) were exported as 0. The generator emits a ConstructUsing for the affected types now, and the deserializer sets non-public setters by reflection. * SystemConfiguration, ConfigurationUpdate, ConfigurationUpdateState and CastleSiegeData were not part of the backup at all. The applied updates matter, because they would be applied again to the restored data. * BinaryAsHexJsonConverter subtracted the "\x" prefix twice, so every byte array which is read from the postgres json lost its last two bytes (e.g. the terrain data of every map). This bug is older than the backup feature, but it made every exported backup lossy. Tests: the in-memory round trip also covers the item slots now, the new BinaryAsHexJsonConverterTests cover the byte array sizes, and BackupServiceEfCoreTests does the round trip against a real database. The latter is ignored by default like the other tests which need a database - it was run locally against postgres 16 and passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LhfyPthjrRZ6K9CgVd26RT
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
2 participants
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.
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/BackupServicein theMUnique.OpenMU.Persistenceproject, without any dependency to the entity framework - it works with the repositories which are available throughIPersistenceContextProvider/IContext.InMemoryBackupService, which supports the export and not the restore.BackupControllerfor the download and the export/import UI on the setup page, including the progress spinner during the import.LocalizedStringJsonConverterin the serializer/deserializer options and theBrowserFileStream->MemoryStreamcopy 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": [ ... ] }), butReferenceResolvingConverteronly accepted plain arrays - the shape the postgres json query produces - and silently skipped everything else. A restoredGameConfigurationtherefore 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 forIElementas the previous commit did. That's what those properties (e.g. of theSkillEntry) actually are, and it also covers the transientIAttributeSystemone, 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. TheMapster.CompileExceptionreported 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 theAdd-method is taken from theICollection<T>interface, because the implementing type may have otherAddoverloads.The import checks the file before the database is re-created.
ContainsRestorableDataverifies 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 theDatabaseInitializedevent of theSetupService. The import on the setup page is the only path now.Additionally: the
Setup.razorconflict with master, some StyleCop warnings (SA1202, SA1204, SA1208) and missing newlines at the end of files.Test
BackupServiceTestsdoes 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