Fix large legacy Site Export import failing with LiteDB FindAll loop - #7379
Fix large legacy Site Export import failing with LiteDB FindAll loop#7379matt468779 wants to merge 1 commit into
Conversation
Importing a legacy Site Export package (LiteDB 3.x on-disk format, produced
by DNN 9.10.2-era Site Export/Import) whose largest collection exceeds
~2,550 records failed in VerifyImportPackage with HTTP 400 "Package is not
valid. Technical Details:Detected loop in FindAll({0})", so the import
never started.
Root cause: ExportImportRepository opens the export database with
Upgrade = true, which forces LiteDB to run a full in-place Rebuild() when
the file is in the legacy v3 format. LiteDB 5.0.21 (the pinned binary
dependency) has a bug in that rebuild path: IndexService.FindAll uses a
MAX_ITEMS_COUNT loop-detection guard initialized as if the target
collection were empty, so any single collection above ~2,550 nodes
falsely throws "Detected loop in FindAll({0})". There is no fixed stable
LiteDB release and it is a binary dependency, so the version cannot be
bumped.
Fix: detect a legacy-format (LiteDB v7) export database by its file
header and, instead of the broken in-place rebuild, migrate it into a
fresh LiteDB 5.x database by streaming every collection's documents
through normal inserts (which do not go through the broken rebuild
FindAll guard), then operate on the migrated copy. Native 5.x databases
and small legacy databases that fail to migrate fall back to the
original Upgrade = true fast path, so unaffected imports keep their exact
previous behavior. The public ExportImportRepository API is unchanged;
the migrated copy is a temporary file cleaned up on Dispose.
Added ExportImportRepositoryTests with genuine LiteDB v3.1.0 fixtures - a
3,000-row database (above the threshold, reproduces the original failure
on the old path) that the repository must now open and fully read, plus
50-row legacy and native 5.x regression cases.
Fix bug dnnsoftware#7378
|
@microsoft-github-policy-service agree company="Trilogy" |
| collection.Update(documentsToUpdate); | ||
| } | ||
|
|
||
| protected virtual void Dispose(bool disposing) |
There was a problem hiding this comment.
Changing Dispose from protected to private is a breaking change
| var engineSettings = Activator.CreateInstance(engineSettingsType); | ||
| engineSettingsType.GetProperty("Filename").SetValue(engineSettings, sourceDbFileName); | ||
|
|
||
| var reader = (IDisposable)Activator.CreateInstance( |
There was a problem hiding this comment.
Could we use the IFileReader type here, so we can avoid some of the reflection?
| } | ||
|
|
||
| var liteDbAssembly = typeof(LiteDatabase).Assembly; | ||
| var engineSettingsType = liteDbAssembly.GetType("LiteDB.Engine.EngineSettings"); |
There was a problem hiding this comment.
EngineSettings is public, do we need to create it via reflection?
|
Thanks, this seems like a reasonable resolution to the issue. However, I'm wanted to get a better understanding of the scenario in which this happens. Why would you be exporting a 9.10.2 site and importing it into a 10.3.x site? Wouldn't it be better to upgrade the site first? I'm wondering if this is a scenario we want to support. If we don't want to support imports across versions, you make a good point that we should give a better error message. Any thoughts @dnnsoftware/approvers? |
|
My $0.02 here is that we shouldn't really support export/import in this manner, as there are so many things that could be a problem, especially 9.x -> 10.x or any other major version crosses given breaking change possibilities. |
|
I do agree that it would be very difficult to support. We should have a message saying something along the lines of "this export cames from a different version and importing it may succeed, completely fail or partially work, we do not recommend import/export between different versions" |
4 participants
Fixes #7378
Summary
Importing a legacy Site Export package (LiteDB 3.x on-disk format, produced by DNN 9.10.2-era Site Export/Import) whose largest collection exceeds ~2,550 records fails
VerifyImportPackagewith HTTP 400Package is not valid. Technical Details:Detected loop in FindAll({0}), so the import never starts.Root cause:
ExportImportRepository(DNN Platform/Modules/DnnExportImportLibrary/Repository/ExportImportRepository.cs) opens the export database withUpgrade = true, which forces LiteDB to run a full in-placeRebuild()when the file is in the legacy v3 on-disk format. LiteDB 5.0.21 (the version pinned inDirectory.Packages.props) has a bug in that rebuild path:IndexService.FindAlluses aMAX_ITEMS_COUNTloop-detection guard initialized as if the target collection were empty, so any single collection above roughly 2,550 records falsely throwsDetected loop in FindAll({0}). This is documented upstream in litedb-org/LiteDB#2525 and litedb-org/LiteDB#2582, with an unmerged fix at litedb-org/LiteDB#2603. Since LiteDB is a binary dependency here and there's no fixed stable release available, bumping the version isn't currently an option.Fix: detect a legacy-format (LiteDB v7) export database by its file header, and instead of the broken in-place rebuild, migrate it into a fresh LiteDB 5.x database by streaming every collection's documents through normal inserts — which don't go through the broken rebuild guard — then operate on the migrated copy. Native 5.x databases and small legacy databases that fail to migrate fall back to the original
Upgrade = truefast path, so unaffected imports keep their exact previous behavior. The publicExportImportRepositoryAPI is unchanged; the migrated copy is a temporary file cleaned up onDispose.Release Note
Fixed an issue where importing a large legacy (DNN 9.10.2-era) Site Export package failed with a LiteDB "Detected loop in FindAll" error, blocking the import before it could start.
Testing Steps
ExportFoldercollection contains more than ~2,550 records (e.g. create 3,000+ folders in the source site before exporting).App_Data/ExportImport/<packageName>/on a DNN Platform 10.3.x instance built from this branch.Detected loop in FindAll({0})).Also added
ExportImportRepositoryTestswith genuine LiteDB v3.1.0 fixtures:legacy_v3_large.dnndb— 3,000ExportFolderrows (above the threshold; reproduces the original failure on the oldUpgrade = truepath — the repository must now open it and read all rows back correctly)legacy_v3_small.dnndb— 50 rows (below the threshold; regression check that small legacy packages keep working)