fix(content): remove superseded curated map/ZIM files when a new version installs
Only Wikipedia had version cleanup; every other curated map and non-Wikipedia ZIM left its prior version on disk when a newer one installed, so users silently accumulated orphaned content (potentially hundreds of GB). (#634) The install paths already record each resource via InstalledResource {resource_id, resource_type, version, file_path}, so the authoritative old-file path for a resource is known. On install of a new version we now capture the prior row before updateOrCreate repoints it, then delete the old file — gated behind a pure, fully unit-tested decision function with strict safety rails: - tracked-only: requires a prior InstalledResource row for the same resource_id, so sideloaded/untracked files are never touched - genuine replacement: old and new file paths must differ - new-file-verified: the new file must be confirmed on disk first - strictly-newer: a re-install or downgrade can't wipe a newer file - within-storage-dir: the old path must resolve under the content store ZIM cleanup deletes the old file directly (NOT via this.delete(), which would drop the InstalledResource row by resource_id that updateOrCreate just repointed) and rebuilds the Kiwix library only if a file was actually removed, so its XML never references a deleted ZIM. Maps need no library step. Wikipedia keeps its own existing cleanup path. All deletions are best-effort and logged; a failure never breaks the install. Closes #634 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2ae30a4abb
commit
bbd62d8ed1
|
|
@ -21,6 +21,7 @@ import logger from '@adonisjs/core/services/logger'
|
|||
import { assertNotPrivateUrl } from '#validators/common'
|
||||
import InstalledResource from '#models/installed_resource'
|
||||
import { CollectionManifestService } from './collection_manifest_service.js'
|
||||
import { decideSupersededDeletion } from '../utils/superseded_resource.js'
|
||||
import type { CollectionWithStatus, MapsSpec } from '../../types/collections.js'
|
||||
import type { Country, CountryCode, CountryGroup, MapExtractPreflight } from '../../types/maps.js'
|
||||
import {
|
||||
|
|
@ -199,10 +200,18 @@ export class MapService implements IMapService {
|
|||
const parsed = CollectionManifestService.parseMapFilename(filename)
|
||||
if (!parsed) continue
|
||||
|
||||
const filepath = join(process.cwd(), this.mapStoragePath, 'pmtiles', filename)
|
||||
const pmtilesDir = join(process.cwd(), this.mapStoragePath, 'pmtiles')
|
||||
const filepath = join(pmtilesDir, filename)
|
||||
const stats = await getFileStatsIfExists(filepath)
|
||||
|
||||
try {
|
||||
// Capture the prior install for this resource_id before updateOrCreate
|
||||
// overwrites it, so we know the old file to clean up (#634).
|
||||
const prior = await InstalledResource.query()
|
||||
.where('resource_id', parsed.resource_id)
|
||||
.where('resource_type', 'map')
|
||||
.first()
|
||||
|
||||
const { DateTime } = await import('luxon')
|
||||
await InstalledResource.updateOrCreate(
|
||||
{ resource_id: parsed.resource_id, resource_type: 'map' },
|
||||
|
|
@ -215,6 +224,31 @@ export class MapService implements IMapService {
|
|||
}
|
||||
)
|
||||
logger.info(`[MapService] Created InstalledResource entry for: ${parsed.resource_id}`)
|
||||
|
||||
// Remove the superseded prior version's pmtiles file if every safety
|
||||
// rail passes (see decideSupersededDeletion). Maps have no library index,
|
||||
// so a direct delete of the recorded old file is sufficient.
|
||||
const decision = decideSupersededDeletion({
|
||||
existing: prior ? { file_path: prior.file_path, version: prior.version } : null,
|
||||
newFilePath: filepath,
|
||||
newVersion: parsed.version,
|
||||
newFileExists: !!stats,
|
||||
storageBaseDir: pmtilesDir,
|
||||
})
|
||||
if (decision.delete && decision.path) {
|
||||
try {
|
||||
await deleteFileIfExists(decision.path)
|
||||
logger.info(
|
||||
`[MapService] Removed superseded ${parsed.resource_id} file: ${decision.path}`
|
||||
)
|
||||
} catch (err) {
|
||||
logger.warn(`[MapService] Failed to remove superseded file ${decision.path}:`, err)
|
||||
}
|
||||
} else if (decision.reason !== 'first_install' && decision.reason !== 'same_file') {
|
||||
logger.info(
|
||||
`[MapService] Kept prior ${parsed.resource_id} file (reason: ${decision.reason})`
|
||||
)
|
||||
}
|
||||
} catch (error) {
|
||||
logger.error(`[MapService] Failed to create InstalledResource for ${filename}:`, error)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import * as cheerio from 'cheerio'
|
|||
import { XMLParser } from 'fast-xml-parser'
|
||||
import { isRawListRemoteZimFilesResponse, isRawRemoteZimFileEntry } from '../../util/zim.js'
|
||||
import { findReplacedWikipediaFiles } from '../utils/zim_filename.js'
|
||||
import { decideSupersededDeletion } from '../utils/superseded_resource.js'
|
||||
import logger from '@adonisjs/core/services/logger'
|
||||
import { DockerService } from './docker_service.js'
|
||||
import { inject } from '@adonisjs/core'
|
||||
|
|
@ -358,6 +359,8 @@ export class ZimService {
|
|||
}
|
||||
|
||||
// Create InstalledResource entries for downloaded files
|
||||
const zimStorageDir = join(process.cwd(), ZIM_STORAGE_PATH)
|
||||
let removedSupersededZim = false
|
||||
for (const url of urls) {
|
||||
// Skip Wikipedia files (managed separately)
|
||||
if (url.includes('wikipedia_en_')) continue
|
||||
|
|
@ -368,10 +371,17 @@ export class ZimService {
|
|||
const parsed = CollectionManifestService.parseZimFilename(filename)
|
||||
if (!parsed) continue
|
||||
|
||||
const filepath = join(process.cwd(), ZIM_STORAGE_PATH, filename)
|
||||
const filepath = join(zimStorageDir, filename)
|
||||
const stats = await getFileStatsIfExists(filepath)
|
||||
|
||||
try {
|
||||
// Capture the prior install for this resource_id BEFORE updateOrCreate
|
||||
// overwrites it, so we know the old file path to clean up (#634).
|
||||
const prior = await InstalledResource.query()
|
||||
.where('resource_id', parsed.resource_id)
|
||||
.where('resource_type', 'zim')
|
||||
.first()
|
||||
|
||||
const { DateTime } = await import('luxon')
|
||||
await InstalledResource.updateOrCreate(
|
||||
{ resource_id: parsed.resource_id, resource_type: 'zim' },
|
||||
|
|
@ -384,10 +394,49 @@ export class ZimService {
|
|||
}
|
||||
)
|
||||
logger.info(`[ZimService] Created InstalledResource entry for: ${parsed.resource_id}`)
|
||||
|
||||
// Remove the superseded prior version's file if (and only if) every
|
||||
// safety rail passes — see decideSupersededDeletion. The InstalledResource
|
||||
// row already points at the new file, so we delete the old file directly
|
||||
// (NOT via this.delete(), which would drop the row by resource_id).
|
||||
const decision = decideSupersededDeletion({
|
||||
existing: prior ? { file_path: prior.file_path, version: prior.version } : null,
|
||||
newFilePath: filepath,
|
||||
newVersion: parsed.version,
|
||||
newFileExists: !!stats,
|
||||
storageBaseDir: zimStorageDir,
|
||||
})
|
||||
if (decision.delete && decision.path) {
|
||||
try {
|
||||
await deleteFileIfExists(decision.path)
|
||||
removedSupersededZim = true
|
||||
logger.info(
|
||||
`[ZimService] Removed superseded ${parsed.resource_id} file: ${decision.path}`
|
||||
)
|
||||
} catch (err) {
|
||||
logger.warn(`[ZimService] Failed to remove superseded file ${decision.path}:`, err)
|
||||
}
|
||||
} else if (decision.reason !== 'first_install' && decision.reason !== 'same_file') {
|
||||
logger.info(
|
||||
`[ZimService] Kept prior ${parsed.resource_id} file (reason: ${decision.reason})`
|
||||
)
|
||||
}
|
||||
} catch (error) {
|
||||
logger.error(`[ZimService] Failed to create InstalledResource for ${filename}:`, error)
|
||||
}
|
||||
}
|
||||
|
||||
// If we removed any superseded ZIM, rebuild the Kiwix library so its XML no
|
||||
// longer references the deleted file. The earlier rebuild in this flow ran
|
||||
// while both versions were still on disk.
|
||||
if (removedSupersededZim) {
|
||||
try {
|
||||
await new KiwixLibraryService().rebuildFromDisk()
|
||||
logger.info('[ZimService] Rebuilt Kiwix library after removing superseded ZIM(s).')
|
||||
} catch (err) {
|
||||
logger.error('[ZimService] Failed to rebuild Kiwix library after cleanup:', err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -0,0 +1,72 @@
|
|||
import { resolve, sep } from 'node:path'
|
||||
|
||||
/**
|
||||
* Decides whether a curated resource's PREVIOUSLY-installed file should be
|
||||
* deleted now that a newer version has been downloaded (issue #634 — old map
|
||||
* and ZIM versions accumulated on disk indefinitely because only Wikipedia had
|
||||
* version cleanup).
|
||||
*
|
||||
* This is intentionally a pure function so every safety rail is unit-testable
|
||||
* without touching the DB or filesystem. The caller looks up the prior
|
||||
* `InstalledResource` row, records the new version, then asks this whether the
|
||||
* old file is safe to remove.
|
||||
*
|
||||
* Safety rails (a "delete" decision requires ALL of these):
|
||||
* - There was a prior install for this exact resource_id (`existing` non-null).
|
||||
* Untracked / sideloaded files have no row and are therefore never touched.
|
||||
* - The old file path actually differs from the new one (a genuine version
|
||||
* swap, not a re-download of the same file).
|
||||
* - The new file is confirmed present on disk — we never remove the old copy
|
||||
* before the replacement is verified.
|
||||
* - The new version is strictly newer than the recorded one, so a re-install
|
||||
* or downgrade can't wipe a newer file.
|
||||
* - The old path resolves to within the resource's storage directory, so a
|
||||
* malformed DB value can't direct a delete outside the content store.
|
||||
*/
|
||||
|
||||
export interface SupersededInputs {
|
||||
/** Prior InstalledResource row for this resource_id, or null on first install. */
|
||||
existing: { file_path: string; version: string } | null
|
||||
/** Absolute path of the newly downloaded file. */
|
||||
newFilePath: string
|
||||
/** Version of the newly downloaded file (e.g. "2026-05"). */
|
||||
newVersion: string
|
||||
/** Whether the new file is confirmed present on disk. */
|
||||
newFileExists: boolean
|
||||
/** Absolute storage directory the old file must live under to be eligible. */
|
||||
storageBaseDir: string
|
||||
}
|
||||
|
||||
export type SupersededReason =
|
||||
| 'first_install'
|
||||
| 'same_file'
|
||||
| 'new_file_missing'
|
||||
| 'not_newer'
|
||||
| 'outside_storage'
|
||||
| 'superseded'
|
||||
|
||||
export interface SupersededDecision {
|
||||
delete: boolean
|
||||
/** Resolved old path to delete — set only when `delete` is true. */
|
||||
path?: string
|
||||
reason: SupersededReason
|
||||
}
|
||||
|
||||
export function decideSupersededDeletion(inputs: SupersededInputs): SupersededDecision {
|
||||
const { existing, newFilePath, newVersion, newFileExists, storageBaseDir } = inputs
|
||||
|
||||
if (!existing) return { delete: false, reason: 'first_install' }
|
||||
if (existing.file_path === newFilePath) return { delete: false, reason: 'same_file' }
|
||||
if (!newFileExists) return { delete: false, reason: 'new_file_missing' }
|
||||
// Versions are zero-padded date strings (YYYY-MM / YYYY-MM-DD), so a lexical
|
||||
// compare orders them correctly. Require strictly newer.
|
||||
if (!(newVersion > existing.version)) return { delete: false, reason: 'not_newer' }
|
||||
|
||||
const resolvedOld = resolve(existing.file_path)
|
||||
const base = resolve(storageBaseDir)
|
||||
if (resolvedOld !== base && !resolvedOld.startsWith(base + sep)) {
|
||||
return { delete: false, reason: 'outside_storage' }
|
||||
}
|
||||
|
||||
return { delete: true, path: resolvedOld, reason: 'superseded' }
|
||||
}
|
||||
|
|
@ -0,0 +1,118 @@
|
|||
import * as assert from 'node:assert/strict'
|
||||
import { test } from 'node:test'
|
||||
import { join } from 'node:path'
|
||||
|
||||
import { decideSupersededDeletion } from '../../app/utils/superseded_resource.js'
|
||||
|
||||
const STORAGE = join('/app', 'storage', 'zim')
|
||||
const OLD = join(STORAGE, 'ifixit_en_all_2026-03.zim')
|
||||
const NEW = join(STORAGE, 'ifixit_en_all_2026-05.zim')
|
||||
|
||||
test('first install (no prior row) deletes nothing', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: null,
|
||||
newFilePath: NEW,
|
||||
newVersion: '2026-05',
|
||||
newFileExists: true,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, false)
|
||||
assert.equal(d.reason, 'first_install')
|
||||
})
|
||||
|
||||
test('superseded prior version is deleted', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: { file_path: OLD, version: '2026-03' },
|
||||
newFilePath: NEW,
|
||||
newVersion: '2026-05',
|
||||
newFileExists: true,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, true)
|
||||
assert.equal(d.reason, 'superseded')
|
||||
assert.equal(d.path, OLD)
|
||||
})
|
||||
|
||||
test('same file path is never deleted (re-download)', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: { file_path: NEW, version: '2026-05' },
|
||||
newFilePath: NEW,
|
||||
newVersion: '2026-05',
|
||||
newFileExists: true,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, false)
|
||||
assert.equal(d.reason, 'same_file')
|
||||
})
|
||||
|
||||
test('old file is NOT deleted until the new file is confirmed on disk', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: { file_path: OLD, version: '2026-03' },
|
||||
newFilePath: NEW,
|
||||
newVersion: '2026-05',
|
||||
newFileExists: false,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, false)
|
||||
assert.equal(d.reason, 'new_file_missing')
|
||||
})
|
||||
|
||||
test('downgrade / reinstall of an older version does not wipe the newer file', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: { file_path: NEW, version: '2026-05' },
|
||||
newFilePath: OLD,
|
||||
newVersion: '2026-03',
|
||||
newFileExists: true,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, false)
|
||||
assert.equal(d.reason, 'not_newer')
|
||||
})
|
||||
|
||||
test('equal version is not considered newer', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: { file_path: join(STORAGE, 'ifixit_en_all_2026-05a.zim'), version: '2026-05' },
|
||||
newFilePath: NEW,
|
||||
newVersion: '2026-05',
|
||||
newFileExists: true,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, false)
|
||||
assert.equal(d.reason, 'not_newer')
|
||||
})
|
||||
|
||||
test('refuses to delete a path outside the storage directory', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: { file_path: '/etc/passwd', version: '2026-03' },
|
||||
newFilePath: NEW,
|
||||
newVersion: '2026-05',
|
||||
newFileExists: true,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, false)
|
||||
assert.equal(d.reason, 'outside_storage')
|
||||
})
|
||||
|
||||
test('refuses a traversal escape that only looks like it is under storage', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: { file_path: join(STORAGE, '..', 'mysql', 'data.ibd'), version: '2026-03' },
|
||||
newFilePath: NEW,
|
||||
newVersion: '2026-05',
|
||||
newFileExists: true,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, false)
|
||||
assert.equal(d.reason, 'outside_storage')
|
||||
})
|
||||
|
||||
test('YYYY-MM-DD versions order correctly against YYYY-MM', () => {
|
||||
const d = decideSupersededDeletion({
|
||||
existing: { file_path: OLD, version: '2026-05' },
|
||||
newFilePath: NEW,
|
||||
newVersion: '2026-05-12',
|
||||
newFileExists: true,
|
||||
storageBaseDir: STORAGE,
|
||||
})
|
||||
assert.equal(d.delete, true)
|
||||
assert.equal(d.reason, 'superseded')
|
||||
})
|
||||
Loading…
Reference in New Issue