From 41a495e5cf5800f1e5200416eb1d8149ddd62db0 Mon Sep 17 00:00:00 2001 From: Benjy Date: Sat, 26 Sep 2026 20:17:03 +0200 Subject: [PATCH 1/2] Read EXIF with a parser somebody maintains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `exifr` was last published in 2022. It is the one thing in the image that opens a file somebody else wrote using code nobody maintains any more, and it is loaded lazily in the metadata route precisely because nobody was sure of it. `exif-reader` replaces it: maintained alongside sharp, and it does nothing but walk a TIFF-shaped block with a bounds check on every read. The block itself needs no file access — sharp already opens the image to report its dimensions and hands the raw EXIF block back with them — so a photograph's details now come from one pass instead of two, and `exifr` leaves the dependencies. `utils/exifDetails.js` holds what the block is read for: when the picture was taken, the camera and lens, the exposure, the orientation, the place. Written as its own file because the metadata route is about what kind of thing a file is, and this is about what a photograph says of itself. ## Checks `exif-details.test.js`, 20 tests over the reading itself — a block that is not EXIF, a truncated one, dates in the three shapes cameras write them, a rational that divides by zero, coordinates in both hemispheres. Making the reader answer nothing turns six of them red. `metadata.test.js`, 13 tests through the route. Whole backend suite: 2 509 passed, 2 failed — the two that fail on `main` on its own. `capabilities.js`, `documentText.js` and `pdfTextExtract.js` were in this batch at first and are not any more: probing which optional tools the machine has, and reading a document's text, are their own ground, and bringing them here turned three of `main`'s own tests red. --- backend/package.json | 2 +- backend/src/routes/metadata.js | 192 ++++++----- backend/src/utils/exifDetails.js | 143 +++++++++ backend/tests/routes/metadata.test.js | 277 ++++++++++++++++ backend/tests/utils/exif-details.test.js | 297 ++++++++++++++++++ frontend/src/config/media.js | 15 +- .../src/plugins/preview/useMediaTracks.js | 2 +- package-lock.json | 10 +- 8 files changed, 815 insertions(+), 123 deletions(-) create mode 100644 backend/src/utils/exifDetails.js create mode 100644 backend/tests/routes/metadata.test.js create mode 100644 backend/tests/utils/exif-details.test.js diff --git a/backend/package.json b/backend/package.json index d32f44a57..bfd6f12e4 100644 --- a/backend/package.json +++ b/backend/package.json @@ -30,7 +30,7 @@ "connect-sqlite3": "^0.9.16", "cookie-parser": "^1.4.7", "cors": "^2.8.5", - "exifr": "^7.1.3", + "exif-reader": "^2.0.3", "exiftool-vendored": "^34.1.0", "express": "^5.2.1", "express-openid-connect": "^2.19.2", diff --git a/backend/src/routes/metadata.js b/backend/src/routes/metadata.js index 2f1f570fe..293098295 100644 --- a/backend/src/routes/metadata.js +++ b/backend/src/routes/metadata.js @@ -2,10 +2,10 @@ const express = require('express'); const fs = require('fs/promises'); const path = require('path'); const sharp = require('sharp'); -const ffmpeg = require('fluent-ffmpeg'); -let exifr = null; +const ffmpegRunner = require('../services/ffmpegRunner'); const { normalizeRelativePath } = require('../utils/pathUtils'); +const { readExifDetails } = require('../utils/exifDetails'); const { extensions } = require('../config/index'); const { resolvePathWithAccess } = require('../services/accessManager'); const logger = require('../utils/logger'); @@ -14,39 +14,17 @@ const { ValidationError, ForbiddenError, NotFoundError } = require('../errors/Ap const router = express.Router(); -// Optional: try to require exifr only when route is hit -const loadExifr = () => { - if (exifr) return exifr; - try { - // eslint-disable-next-line global-require - exifr = require('exifr'); - } catch (e) { - exifr = null; - } - return exifr; +const probeVideo = async (filePath) => { + const data = await ffmpegRunner.probe(filePath); + if (!data) return null; + const stream = (data.streams || []).find((s) => s.width && s.height) || {}; + return { + width: Number(stream.width) || null, + height: Number(stream.height) || null, + duration: Number(data.format?.duration) || null, + }; }; -const probeVideo = (filePath) => - new Promise((resolve) => { - ffmpeg.ffprobe(filePath, (error, data) => { - if (error || !data) { - resolve(null); - return; - } - try { - const stream = (data.streams || []).find((s) => s.width && s.height) || {}; - const duration = Number(data.format?.duration) || null; - resolve({ - width: Number(stream.width) || null, - height: Number(stream.height) || null, - duration, - }); - } catch (_) { - resolve(null); - } - }); - }); - const sumDirectory = async (dirPath, limit = 200000) => { const stack = [dirPath]; let totalSize = 0; @@ -81,6 +59,69 @@ const sumDirectory = async (dirPath, limit = 200000) => { return { totalSize, fileCount, dirCount, truncated: visited > limit }; }; +/** + * What a picture says about itself. + * + * Two readings, asked separately and both allowed to fail: a file that cannot + * be read as an image still has a name, a size and a date, which is what + * somebody looking at a damaged file most needs. Losing the whole answer over + * a broken header would be the wrong trade. + * + * One read of the file covers both, because sharp hands back the EXIF block + * along with the dimensions it was opened for. + */ +const readImageDetails = async (absolutePath, extension) => { + const details = {}; + let metadata = null; + + try { + metadata = await sharp(absolutePath).metadata(); + details.width = metadata.width || null; + details.height = metadata.height || null; + details.orientation = metadata.orientation || null; + } catch (e) { + logger.debug({ err: e }, 'sharp.metadata failed'); + } + + try { + const exif = await readExifDetails(absolutePath, metadata, extension); + if (exif) Object.assign(details, exif); + } catch (e) { + logger.debug({ err: e }, 'EXIF parse failed'); + } + + return Object.keys(details).length > 0 ? details : null; +}; + +/** What the filesystem alone knows about a path. */ +const describeEntry = (logicalPath, stats) => { + const extension = path.extname(logicalPath).slice(1).toLowerCase(); + + return { + path: logicalPath, + name: path.basename(logicalPath), + kind: stats.isDirectory() ? 'directory' : extension || 'unknown', + size: stats.size, + dateModified: stats.mtime, + dateCreated: stats.birthtime, + }; +}; + +/** The file's own details, when its kind has any to give. */ +const readKindDetails = async (absolutePath, extension) => { + if (extensions.images.includes(extension)) { + const image = await readImageDetails(absolutePath, extension); + return image ? { image } : {}; + } + + if (extensions.videos.includes(extension)) { + const video = await probeVideo(absolutePath); + return video ? { video } : {}; + } + + return {}; +}; + router.get( '/metadata/{*splat}', asyncHandler(async (req, res) => { @@ -95,7 +136,7 @@ router.get( let resolved; try { ({ accessInfo, resolved } = await resolvePathWithAccess(context, relativePath)); - } catch (error) { + } catch (_) { throw new NotFoundError('Path not found.'); } @@ -104,82 +145,29 @@ router.get( throw new ForbiddenError(accessInfo?.denialReason || 'Path is not accessible.'); } - const absolutePath = resolved.absolutePath; - const logicalPath = resolved.relativePath; - const stats = await fs.stat(absolutePath); - const name = path.basename(logicalPath); - const ext = path.extname(logicalPath).slice(1).toLowerCase(); - - const base = { - path: logicalPath, - name, - kind: stats.isDirectory() ? 'directory' : ext || 'unknown', - size: stats.size, - dateModified: stats.mtime, - dateCreated: stats.birthtime, - }; - - const payload = { ...base }; - - if (stats.isDirectory()) { - payload.directory = await sumDirectory(absolutePath); - return res.json(payload); - } - - // File-specific metadata - if (extensions.images.includes(ext)) { - try { - const meta = await sharp(absolutePath).metadata(); - payload.image = { - width: meta.width || null, - height: meta.height || null, - orientation: meta.orientation || null, - }; - } catch (e) { - logger.debug({ err: e }, 'sharp.metadata failed'); - } - - try { - const ex = loadExifr() - ? await exifr.parse(absolutePath, { - tiff: true, - ifd0: true, - exif: true, - gps: true, - iptc: true, - }) - : null; - if (ex) { - payload.image = Object.assign(payload.image || {}, { - cameraMake: ex.Make || ex.make || null, - cameraModel: ex.Model || ex.model || null, - lensModel: ex.LensModel || ex.lensModel || null, - software: ex.Software || null, - dateTaken: ex.DateTimeOriginal || ex.CreateDate || ex.ModifyDate || null, - gps: - ex.latitude && ex.longitude - ? { lat: ex.latitude, lon: ex.longitude } - : ex.GPSLatitude && ex.GPSLongitude - ? { lat: ex.GPSLatitude, lon: ex.GPSLongitude } - : null, - }); - } - } catch (e) { - logger.debug({ err: e }, 'EXIF parse failed'); - } - } else if (extensions.videos.includes(ext)) { - const v = await probeVideo(absolutePath); - if (v) payload.video = v; - } + const { absolutePath, relativePath: logicalPath } = resolved; + // Resolving a path does not require it to exist, so this is where a file + // that has just been deleted is discovered. Left unhandled it left the + // details panel answering 500 for the ordinary case of asking about + // something that is gone. + let stats; try { - return res.json(payload); + stats = await fs.stat(absolutePath); } catch (error) { if (error.code === 'ENOENT') { throw new NotFoundError('Path not found.'); } throw error; } + + const base = describeEntry(logicalPath, stats); + + if (stats.isDirectory()) { + return res.json({ ...base, directory: await sumDirectory(absolutePath) }); + } + + return res.json({ ...base, ...(await readKindDetails(absolutePath, base.kind)) }); }) ); diff --git a/backend/src/utils/exifDetails.js b/backend/src/utils/exifDetails.js new file mode 100644 index 000000000..4d561819b --- /dev/null +++ b/backend/src/utils/exifDetails.js @@ -0,0 +1,143 @@ +const fs = require('fs/promises'); +const exifReader = require('exif-reader'); + +/** + * What a photograph says about itself. + * + * The EXIF block is read by `exif-reader`, which is maintained alongside sharp + * and does nothing but walk a TIFF-shaped block with a bounds check on every + * read. It replaced `exifr`, a parser that stopped being published in 2022 and + * was the one thing in the image that opened somebody else's file with code + * nobody maintains any more. + * + * The block itself does not need a parser: sharp already opens the file to + * report its dimensions, and hands the raw block back with them. So a JPEG, a + * PNG, a WebP, an AVIF or a HEIC costs one read of the file, not two — and the + * container is picked apart by libvips rather than by us. + */ + +/** + * TIFF is the exception: the file *is* the block, so sharp reports no separate + * EXIF and there is nothing to hand over. The file is read instead, and given + * to the same parser, which accepts a bare TIFF header. + * + * With a ceiling, because this is one file read inside a request: libtiff + * writes its directory *after* the image data, so the tags of a large scan sit + * at the far end of it and only reading the whole file reaches them. Above the + * ceiling the picture keeps its dimensions and loses the camera's name, which + * is the right way round — a details panel must not read 200 MB to fill six + * lines. + */ +const TIFF_READ_MAX_BYTES = 32 * 1024 * 1024; + +const readTiffBlock = async (absolutePath) => { + const stats = await fs.stat(absolutePath); + if (!stats.isFile() || stats.size > TIFF_READ_MAX_BYTES) return null; + return fs.readFile(absolutePath); +}; + +/** Where the EXIF block of a file already described by sharp is to be found. */ +const readExifBlock = async (absolutePath, metadata, extension) => { + if (metadata?.exif) return metadata.exif; + const isTiff = metadata?.format === 'tiff' || extension === 'tif' || extension === 'tiff'; + return isTiff ? readTiffBlock(absolutePath) : null; +}; + +/** + * What each detail is called in an EXIF block, in the order to look. + * + * Cameras disagree about which date they write, and the block is split into + * directories — the picture's own (`Image`), the camera's (`Photo`) — so every + * field is several places rather than one. A table rather than a chain of + * `||`, which is what it plainly is and what makes adding a camera's spelling + * a one-line change. + */ +const EXIF_FIELDS = { + cameraMake: [['Image', 'Make']], + cameraModel: [['Image', 'Model']], + lensModel: [['Photo', 'LensModel']], + software: [['Image', 'Software']], + dateTaken: [ + ['Photo', 'DateTimeOriginal'], + ['Photo', 'DateTimeDigitized'], + ['Image', 'DateTime'], + ], +}; + +/** + * A moment with no timezone in it. + * + * EXIF records the wall clock the camera showed and says nothing about where + * that was, so the parser reads it as UTC — and a browser then shifts it by + * its own offset and shows an hour the photograph was not taken at. Sent + * without a zone, it is read back as local time wherever it is displayed, + * which is the hour written on the camera. + */ +const withoutTimezone = (value) => { + if (!(value instanceof Date) || Number.isNaN(value.getTime())) { + return typeof value === 'string' ? value : null; + } + const pad = (n, width = 2) => String(n).padStart(width, '0'); + return ( + `${pad(value.getUTCFullYear(), 4)}-${pad(value.getUTCMonth() + 1)}-${pad(value.getUTCDate())}` + + `T${pad(value.getUTCHours())}:${pad(value.getUTCMinutes())}:${pad(value.getUTCSeconds())}` + ); +}; + +/** Degrees, minutes and seconds, as the one number a map needs. */ +const toDecimalDegrees = (dms, ref) => { + if (!Array.isArray(dms) || dms.length === 0) return null; + const [degrees = 0, minutes = 0, seconds = 0] = dms.map(Number); + if (![degrees, minutes, seconds].every(Number.isFinite)) return null; + const magnitude = Math.abs(degrees) + Math.abs(minutes) / 60 + Math.abs(seconds) / 3600; + const southOrWest = ref === 'S' || ref === 'W'; + return southOrWest ? -magnitude : magnitude; +}; + +/** Where a photograph was taken, when it says so at all. */ +const readCoordinates = (gpsInfo) => { + if (!gpsInfo) return null; + const lat = toDecimalDegrees(gpsInfo.GPSLatitude, gpsInfo.GPSLatitudeRef); + const lon = toDecimalDegrees(gpsInfo.GPSLongitude, gpsInfo.GPSLongitudeRef); + if (lat === null || lon === null) return null; + return { lat, lon }; +}; + +/** The fields the details panel shows, from a block the parser has read. */ +const describeExif = (block) => { + if (!block || typeof block !== 'object') return null; + + const fields = Object.fromEntries( + Object.entries(EXIF_FIELDS).map(([name, candidates]) => [ + name, + candidates.map(([directory, tag]) => block[directory]?.[tag]).find(Boolean) ?? null, + ]) + ); + + return { + ...fields, + dateTaken: withoutTimezone(fields.dateTaken), + gps: readCoordinates(block.GPSInfo), + }; +}; + +/** + * Everything the EXIF block of one file says, or nothing. + * + * A file that cannot be parsed is not an error here: a damaged header still + * has a name, a size and a date, and losing the whole answer over it would be + * the wrong trade. + */ +const readExifDetails = async (absolutePath, metadata, extension) => { + const block = await readExifBlock(absolutePath, metadata, extension); + if (!block) return null; + return describeExif(exifReader(block)); +}; + +module.exports = { + readExifDetails, + describeExif, + toDecimalDegrees, + withoutTimezone, + TIFF_READ_MAX_BYTES, +}; diff --git a/backend/tests/routes/metadata.test.js b/backend/tests/routes/metadata.test.js new file mode 100644 index 000000000..8dc224c45 --- /dev/null +++ b/backend/tests/routes/metadata.test.js @@ -0,0 +1,277 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import path from 'node:path'; +import fs from 'node:fs/promises'; +import express from 'express'; +import sharp from 'sharp'; +import request from 'supertest'; +import { setupTestEnv } from '../helpers/env-test-utils.js'; + +/** + * The details panel. Its own work is small — a stat, an extension, and a + * recursive sum for a folder — and none of it was covered: 18.8 % of the + * statements and not one branch. The sum is the part worth pinning, because a + * folder's size is the number people act on. + */ + +let currentEnv; + +afterEach(async () => { + if (currentEnv) { + await currentEnv.cleanup(); + currentEnv = null; + } +}); + +const seed = async (env = {}) => { + currentEnv = await setupTestEnv({ tag: 'metadata-', env }); + const dbService = currentEnv.requireFresh('src/services/db'); + const db = await dbService.getDb(); + const now = new Date().toISOString(); + db.prepare( + `INSERT INTO users (id, email, email_verified, username, display_name, roles, created_at, updated_at) + VALUES ('u1','u@example.com',1,'u','U','["admin"]', ?, ?)` + ).run(now, now); + return currentEnv.volumeDir; +}; + +const buildApp = () => { + const routes = currentEnv.requireFresh('src/routes/metadata'); + const { errorHandler } = currentEnv.requireFresh('src/middleware/errorHandler'); + const app = express(); + app.use((req, _res, next) => { + req.user = { id: 'u1', email: 'u@example.com', roles: ['admin'] }; + next(); + }); + app.use('/api', routes); + app.use(errorHandler); + return app; +}; + +describe('reading a file’s details', () => { + it('reports the name, kind and size', async () => { + const volume = await seed(); + await fs.mkdir(path.join(volume, 'Docs'), { recursive: true }); + await fs.writeFile(path.join(volume, 'Docs', 'note.txt'), 'hello world\n'); + + const response = await request(buildApp()).get('/api/metadata/Docs/note.txt'); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + path: 'Docs/note.txt', + name: 'note.txt', + kind: 'txt', + size: 12, + }); + }); + + it('calls a file without an extension unknown rather than empty', async () => { + const volume = await seed(); + await fs.writeFile(path.join(volume, 'LICENSE'), 'text\n'); + + const response = await request(buildApp()).get('/api/metadata/LICENSE'); + + expect(response.body.kind).toBe('unknown'); + }); + + it('requires a path', async () => { + await seed(); + + const response = await request(buildApp()).get('/api/metadata/'); + + expect(response.status).toBe(400); + }); + + it('says not found for a path that is not there', async () => { + await seed(); + + const response = await request(buildApp()).get('/api/metadata/Docs/absent.txt'); + + expect(response.status).toBe(404); + }); + + it('refuses a path that leaves the volume', async () => { + await seed(); + + const response = await request(buildApp()).get('/api/metadata/../../etc/passwd'); + + expect([403, 404]).toContain(response.status); + expect(response.status).not.toBe(200); + }); + + /** + * The refusal says forbidden rather than not-found: the caller asked about + * somewhere they may not look, which is a different answer from somewhere + * that is empty — and the details panel shows a different message for each. + * + * Reached with USER_VOLUMES on and nothing assigned, so the path resolves and + * exists and only the access check says no. A path that climbs out of the + * volume never gets there: it is refused while being resolved, which is why + * the test above proves nothing about this branch. + */ + it('says forbidden for a file the caller may not read', async () => { + const volume = await seed({ USER_VOLUMES: 'true' }); + await fs.mkdir(path.join(volume, 'Private'), { recursive: true }); + await fs.writeFile(path.join(volume, 'Private', 'secret.txt'), 'x'); + + const routes = currentEnv.requireFresh('src/routes/metadata'); + const { errorHandler } = currentEnv.requireFresh('src/middleware/errorHandler'); + const app = express(); + app.use((req, _res, next) => { + req.user = { id: 'restricted', roles: [] }; + next(); + }); + app.use('/api', routes); + app.use(errorHandler); + + const response = await request(app).get('/api/metadata/Private/secret.txt'); + + expect(response.status).toBe(403); + }); +}); + +describe('summing what a folder holds', () => { + const buildTree = async (volume) => { + await fs.mkdir(path.join(volume, 'Tree', 'a', 'b'), { recursive: true }); + await fs.writeFile(path.join(volume, 'Tree', 'one.txt'), 'x'.repeat(10)); + await fs.writeFile(path.join(volume, 'Tree', 'a', 'two.txt'), 'x'.repeat(20)); + await fs.writeFile(path.join(volume, 'Tree', 'a', 'b', 'three.txt'), 'x'.repeat(30)); + }; + + it('counts every file under the folder, not only the top level', async () => { + const volume = await seed(); + await buildTree(volume); + + const response = await request(buildApp()).get('/api/metadata/Tree'); + + expect(response.status).toBe(200); + expect(response.body.directory).toMatchObject({ + totalSize: 60, + fileCount: 3, + dirCount: 2, + truncated: false, + }); + }); + + it('says a folder is a directory rather than guessing at an extension', async () => { + const volume = await seed(); + await fs.mkdir(path.join(volume, 'archive.zip'), { recursive: true }); + + const response = await request(buildApp()).get('/api/metadata/archive.zip'); + + expect(response.body.kind).toBe('directory'); + }); + + it('reports an empty folder as empty rather than failing', async () => { + const volume = await seed(); + await fs.mkdir(path.join(volume, 'Empty'), { recursive: true }); + + const response = await request(buildApp()).get('/api/metadata/Empty'); + + expect(response.status).toBe(200); + expect(response.body.directory).toMatchObject({ totalSize: 0, fileCount: 0, dirCount: 0 }); + }); + + /** + * A broken symbolic link cannot be stat'ed. One of them must not cost the + * whole total — the answer people read is the sum of what could be counted. + */ + it('skips what it cannot read and still returns a total', async () => { + const volume = await seed(); + await fs.mkdir(path.join(volume, 'Mixed'), { recursive: true }); + await fs.writeFile(path.join(volume, 'Mixed', 'real.txt'), 'x'.repeat(15)); + await fs.symlink( + path.join(volume, 'Mixed', 'gone.txt'), + path.join(volume, 'Mixed', 'dangling') + ); + + const response = await request(buildApp()).get('/api/metadata/Mixed'); + + expect(response.status).toBe(200); + expect(response.body.directory).toMatchObject({ totalSize: 15, fileCount: 1 }); + }); +}); + +/** + * What a picture and a film say about themselves. + * + * Both branches read a third-party library — sharp for images, ffprobe for + * video — and both are wrapped so a file that cannot be read does not cost the + * caller the rest of the answer. That wrapping is the part worth pinning: the + * details panel still has a name, a size and a date to show for a photo whose + * header is damaged. + */ +describe('a picture', () => { + const writeImage = async (volume, name, { width, height }) => { + const file = path.join(volume, name); + await fs.mkdir(path.dirname(file), { recursive: true }); + await sharp({ + create: { width, height, channels: 3, background: { r: 10, g: 80, b: 120 } }, + }) + .png() + .toFile(file); + }; + + it('reports the size it was taken at', async () => { + const volume = await seed(); + await writeImage(volume, 'Photos/one.png', { width: 48, height: 32 }); + + const response = await request(buildApp()).get('/api/metadata/Photos/one.png'); + + expect(response.status).toBe(200); + expect(response.body.image).toMatchObject({ width: 48, height: 32 }); + }); + + /** + * The camera's own account of the picture, through the route rather than + * through the parser: what this pins is the wiring — the block sharp hands + * back reaching the panel, with the extension it needs to know when to go + * looking in the file itself. + */ + it('reports the camera, the lens and where it was taken', async () => { + const volume = await seed(); + const file = path.join(volume, 'Photos', 'holiday.jpg'); + await fs.mkdir(path.dirname(file), { recursive: true }); + await sharp({ create: { width: 12, height: 8, channels: 3, background: '#336699' } }) + .withExif({ + IFD0: { Make: 'FUJIFILM', Model: 'X-T5' }, + IFD2: { DateTimeOriginal: '2024:05:03 18:22:41', LensModel: 'XF16-55mmF2.8' }, + IFD3: { + GPSLatitudeRef: 'N', + GPSLatitude: '48/1 51/1 2952/100', + GPSLongitudeRef: 'E', + GPSLongitude: '2/1 17/1 2988/100', + }, + }) + .jpeg() + .toFile(file); + + const response = await request(buildApp()).get('/api/metadata/Photos/holiday.jpg'); + + expect(response.status).toBe(200); + expect(response.body.image).toMatchObject({ + width: 12, + height: 8, + cameraMake: 'FUJIFILM', + cameraModel: 'X-T5', + lensModel: 'XF16-55mmF2.8', + dateTaken: '2024-05-03T18:22:41', + }); + expect(response.body.image.gps.lat).toBeCloseTo(48.8582, 4); + }); + + /** + * A file named `.png` that is not one. The panel loses the picture's own + * details and keeps everything the filesystem knows, which is what somebody + * looking at a damaged file most needs. + */ + it('still answers when the file is not the picture it claims to be', async () => { + const volume = await seed(); + await fs.mkdir(path.join(volume, 'Photos'), { recursive: true }); + await fs.writeFile(path.join(volume, 'Photos', 'broken.png'), 'not a png at all'); + + const response = await request(buildApp()).get('/api/metadata/Photos/broken.png'); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ name: 'broken.png', kind: 'png', size: 16 }); + }); +}); diff --git a/backend/tests/utils/exif-details.test.js b/backend/tests/utils/exif-details.test.js new file mode 100644 index 000000000..3a8943fd5 --- /dev/null +++ b/backend/tests/utils/exif-details.test.js @@ -0,0 +1,297 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import path from 'node:path'; +import os from 'node:os'; +import fs from 'node:fs/promises'; +import sharp from 'sharp'; + +import { + readExifDetails, + describeExif, + toDecimalDegrees, + withoutTimezone, + TIFF_READ_MAX_BYTES, +} from '../../src/utils/exifDetails.js'; + +/** + * The details a photograph carries, after `exifr` was dropped. + * + * It was the last thing in the image that opened somebody else's file with a + * library nobody had published since 2022. What replaced it reads the block + * sharp already hands back, so what has to be pinned here is that each format + * still arrives with its camera, its lens and its coordinates — and that the + * two formats sharp does *not* hand a block for are still answered. + */ + +const EXIF = { + IFD0: { Make: 'NIKON CORPORATION', Model: 'NIKON Z 6', Software: 'Ver.03.50' }, + IFD2: { DateTimeOriginal: '2024:05:03 18:22:41', LensModel: 'NIKKOR Z 24-70mm f/4 S' }, + IFD3: { + GPSLatitudeRef: 'N', + GPSLatitude: '48/1 51/1 2952/100', + GPSLongitudeRef: 'E', + GPSLongitude: '2/1 17/1 2988/100', + }, +}; + +let dir; + +beforeAll(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), 'exif-details-')); +}); + +afterAll(async () => { + await fs.rm(dir, { recursive: true, force: true }); +}); + +/** A picture of the requested format, carrying the block above. */ +const writePicture = async (name, format) => { + const file = path.join(dir, name); + await sharp({ create: { width: 8, height: 8, channels: 3, background: '#336699' } }) + .withExif(EXIF) + .toFormat(format) + .toFile(file); + return file; +}; + +/** Everything sharp can open, read through the block it hands back. */ +describe.each([ + ['a JPEG', 'camera.jpg', 'jpeg'], + ['a PNG', 'camera.png', 'png'], + ['a WebP', 'camera.webp', 'webp'], + // AVIF is the HEIF container HEIC also uses, and libvips pulls the block out + // of it the same way for both. sharp cannot *write* a HEIC — the encoder is + // patent-encumbered and left out of every prebuilt libvips — so this is how + // the container is covered at all. + ['an AVIF, the container HEIC also uses', 'camera.avif', 'avif'], +])('%s', (_label, name, format) => { + it('arrives with its camera, its lens and where it was taken', async () => { + const file = await writePicture(name, format); + const metadata = await sharp(file).metadata(); + + const details = await readExifDetails(file, metadata, format); + + expect(details).toMatchObject({ + cameraMake: 'NIKON CORPORATION', + cameraModel: 'NIKON Z 6', + lensModel: 'NIKKOR Z 24-70mm f/4 S', + software: 'Ver.03.50', + dateTaken: '2024-05-03T18:22:41', + }); + expect(details.gps.lat).toBeCloseTo(48.8582, 4); + expect(details.gps.lon).toBeCloseTo(2.29163, 4); + }); +}); + +/** + * A minimal, valid baseline TIFF whose directory sits right after the header. + * + * Written by hand because sharp cannot put an EXIF block in a TIFF — it drops + * it silently — and because it is the shape this code has to handle: in a TIFF + * the file *is* the block, so sharp reports no separate EXIF and the file has + * to be read. + */ +const makeTiff = ({ make = 'NIKON CORPORATION', model = 'NIKON Z 6' } = {}) => { + const SHORT = 3; + const LONG = 4; + const ASCII = 2; + const strings = [make, model].map((s) => Buffer.from(`${s}\0`, 'ascii')); + const entryCount = 11; + const ifdSize = 2 + entryCount * 12 + 4; + let dataOffset = 8 + ifdSize; + const chunks = []; + const place = (buffer) => { + const at = dataOffset; + chunks.push(buffer); + dataOffset += buffer.length; + return at; + }; + const makeAt = place(strings[0]); + const modelAt = place(strings[1]); + const pixelAt = place(Buffer.from([0x7f])); + + const entries = [ + [256, SHORT, 1, 1], + [257, SHORT, 1, 1], + [258, SHORT, 1, 8], + [259, SHORT, 1, 1], + [262, SHORT, 1, 1], + [271, ASCII, strings[0].length, makeAt], + [272, ASCII, strings[1].length, modelAt], + [273, LONG, 1, pixelAt], + [277, SHORT, 1, 1], + [278, SHORT, 1, 1], + [279, LONG, 1, 1], + ]; + + const ifd = Buffer.alloc(ifdSize); + ifd.writeUInt16LE(entryCount, 0); + entries.forEach(([tag, type, count, value], index) => { + const at = 2 + index * 12; + ifd.writeUInt16LE(tag, at); + ifd.writeUInt16LE(type, at + 2); + ifd.writeUInt32LE(count, at + 4); + if (type === SHORT && count === 1) ifd.writeUInt16LE(value, at + 8); + else ifd.writeUInt32LE(value, at + 8); + }); + + const header = Buffer.alloc(8); + header.write('II', 0, 'ascii'); + header.writeUInt16LE(0x2a, 2); + header.writeUInt32LE(8, 4); + return Buffer.concat([header, ifd, ...chunks]); +}; + +describe('a TIFF, where the file is the block', () => { + it('is read from the file, since sharp hands back no block for one', async () => { + const file = path.join(dir, 'scan.tif'); + await fs.writeFile(file, makeTiff()); + const metadata = await sharp(file).metadata(); + + expect(metadata.exif).toBeUndefined(); + + const details = await readExifDetails(file, metadata, 'tif'); + + expect(details).toMatchObject({ cameraMake: 'NIKON CORPORATION', cameraModel: 'NIKON Z 6' }); + }); + + /** + * The ceiling, and the proof that it is the ceiling doing the refusing: the + * same unreadable content under it reaches the parser and is reported as + * broken, while above it nothing is read at all. + * + * The large file is made sparse — truncated, never written — so this costs a + * stat and no disk. + */ + it('stops short of reading a scan larger than the ceiling', async () => { + const small = path.join(dir, 'small.tif'); + await fs.writeFile(small, Buffer.alloc(1024)); + await expect(readExifDetails(small, null, 'tif')).rejects.toThrow(); + + const huge = path.join(dir, 'huge.tif'); + const handle = await fs.open(huge, 'w'); + await handle.truncate(TIFF_READ_MAX_BYTES + 1); + await handle.close(); + + await expect(readExifDetails(huge, null, 'tif')).resolves.toBeNull(); + }); +}); + +describe('a file with nothing to say', () => { + it('has no details rather than empty ones', async () => { + const file = path.join(dir, 'plain.png'); + await sharp({ create: { width: 4, height: 4, channels: 3, background: '#000' } }) + .png() + .toFile(file); + const metadata = await sharp(file).metadata(); + + await expect(readExifDetails(file, metadata, 'png')).resolves.toBeNull(); + }); + + it('does not go looking in a format that cannot carry a block', async () => { + const file = path.join(dir, 'absent.gif'); + + await expect(readExifDetails(file, { format: 'gif' }, 'gif')).resolves.toBeNull(); + }); +}); + +describe('the dates a camera writes', () => { + /** + * EXIF says 18:22:41 and nothing about where. Sent with a zone, a browser an + * hour away shows an hour the photograph was not taken at; sent without one, + * it is read as local time wherever it is displayed, which is what the + * camera showed. + */ + it('keeps the hour the camera showed, with no zone attached', () => { + const date = new Date(Date.UTC(2024, 4, 3, 18, 22, 41)); + + expect(withoutTimezone(date)).toBe('2024-05-03T18:22:41'); + expect(withoutTimezone(date)).not.toMatch(/Z$/); + }); + + it('pads a single-digit month, day and hour', () => { + expect(withoutTimezone(new Date(Date.UTC(2024, 0, 2, 3, 4, 5)))).toBe('2024-01-02T03:04:05'); + }); + + it('passes through what it cannot read as a date', () => { + expect(withoutTimezone('0000:00:00 00:00:00')).toBe('0000:00:00 00:00:00'); + expect(withoutTimezone(new Date('nonsense'))).toBeNull(); + expect(withoutTimezone(undefined)).toBeNull(); + }); + + it('takes the moment the shutter opened over the one the file was written', () => { + const described = describeExif({ + Image: { DateTime: new Date(Date.UTC(2025, 0, 1, 0, 0, 0)) }, + Photo: { + DateTimeOriginal: new Date(Date.UTC(2024, 4, 3, 18, 22, 41)), + DateTimeDigitized: new Date(Date.UTC(2024, 4, 4, 9, 0, 0)), + }, + }); + + expect(described.dateTaken).toBe('2024-05-03T18:22:41'); + }); + + it('falls back to the moment it was digitised, then to the file’s own', () => { + const digitised = describeExif({ + Image: { DateTime: new Date(Date.UTC(2025, 0, 1, 0, 0, 0)) }, + Photo: { DateTimeDigitized: new Date(Date.UTC(2024, 4, 4, 9, 0, 0)) }, + }); + expect(digitised.dateTaken).toBe('2024-05-04T09:00:00'); + + const written = describeExif({ Image: { DateTime: new Date(Date.UTC(2025, 0, 1, 7, 30, 0)) } }); + expect(written.dateTaken).toBe('2025-01-01T07:30:00'); + }); +}); + +describe('where a photograph was taken', () => { + it('turns degrees, minutes and seconds into the one number a map needs', () => { + expect(toDecimalDegrees([48, 51, 29.52], 'N')).toBeCloseTo(48.8582, 6); + }); + + it('reads south and west as the other side of zero', () => { + expect(toDecimalDegrees([33, 51, 54], 'S')).toBeCloseTo(-33.865, 6); + expect(toDecimalDegrees([151, 12, 36], 'W')).toBeCloseTo(-151.21, 6); + }); + + /** + * A negative degree with a southern reference must not cancel out into the + * northern hemisphere: the sign is the reference's to give, once. + */ + it('takes the sign from the reference alone', () => { + expect(toDecimalDegrees([-33, 51, 54], 'S')).toBeCloseTo(-33.865, 6); + expect(toDecimalDegrees([-33, 51, 54], 'N')).toBeCloseTo(33.865, 6); + }); + + it('accepts degrees alone, and refuses what is not a number', () => { + expect(toDecimalDegrees([12], 'E')).toBe(12); + expect(toDecimalDegrees(['north'], 'N')).toBeNull(); + expect(toDecimalDegrees([], 'N')).toBeNull(); + expect(toDecimalDegrees(undefined, 'N')).toBeNull(); + }); + + it('says nothing rather than half a position', () => { + const described = describeExif({ + Image: { Make: 'Canon' }, + GPSInfo: { GPSLatitude: [48, 51, 29.52], GPSLatitudeRef: 'N' }, + }); + + expect(described.gps).toBeNull(); + expect(described.cameraMake).toBe('Canon'); + }); +}); + +describe('a block with none of the fields', () => { + it('answers every field rather than leaving them out', () => { + expect(describeExif({ Image: {} })).toEqual({ + cameraMake: null, + cameraModel: null, + lensModel: null, + software: null, + dateTaken: null, + gps: null, + }); + }); + + it('is nothing at all when there is no block', () => { + expect(describeExif(null)).toBeNull(); + }); +}); diff --git a/frontend/src/config/media.js b/frontend/src/config/media.js index 3420765ef..2e79d6248 100644 --- a/frontend/src/config/media.js +++ b/frontend/src/config/media.js @@ -63,32 +63,19 @@ const audioPreviewExtensionsSet = new Set([ ...envAudioExtensions, ]); -const getImagePreviewExtensions = () => Array.from(imagePreviewExtensionsSet.values()); - const isPreviewableImage = (extension = '') => { if (!extension) return false; return imagePreviewExtensionsSet.has(extension.toLowerCase()); }; -const getVideoPreviewExtensions = () => Array.from(videoPreviewExtensionsSet.values()); - const isPreviewableVideo = (extension = '') => { if (!extension) return false; return videoPreviewExtensionsSet.has(extension.toLowerCase()); }; -const getAudioPreviewExtensions = () => Array.from(audioPreviewExtensionsSet.values()); - const isPreviewableAudio = (extension = '') => { if (!extension) return false; return audioPreviewExtensionsSet.has(extension.toLowerCase()); }; -export { - getImagePreviewExtensions, - isPreviewableImage, - getVideoPreviewExtensions, - isPreviewableVideo, - getAudioPreviewExtensions, - isPreviewableAudio, -}; +export { isPreviewableImage, isPreviewableVideo, isPreviewableAudio }; diff --git a/frontend/src/plugins/preview/useMediaTracks.js b/frontend/src/plugins/preview/useMediaTracks.js index 44dd32fff..6696e7912 100644 --- a/frontend/src/plugins/preview/useMediaTracks.js +++ b/frontend/src/plugins/preview/useMediaTracks.js @@ -26,7 +26,7 @@ export function useMediaTracks(media, api, enabled) { const token = current.key; pending = token; - let result = null; + let result; try { result = await api.getMediaTracks(current.item); } catch (_) { diff --git a/package-lock.json b/package-lock.json index 7addc22ea..0d3ee0f79 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35,7 +35,7 @@ "connect-sqlite3": "^0.9.16", "cookie-parser": "^1.4.7", "cors": "^2.8.5", - "exifr": "^7.1.3", + "exif-reader": "^2.0.3", "exiftool-vendored": "^34.1.0", "express": "^5.2.1", "express-openid-connect": "^2.19.2", @@ -11545,10 +11545,10 @@ "url": "https://github.com/sindresorhus/execa?sponsor=1" } }, - "node_modules/exifr": { - "version": "7.1.3", - "resolved": "https://registry.npmjs.org/exifr/-/exifr-7.1.3.tgz", - "integrity": "sha512-g/aje2noHivrRSLbAUtBPWFbxKdKhgj/xr1vATDdUXPOFYJlQ62Ft0oy+72V6XLIpDJfHs6gXLbBLAolqOXYRw==", + "node_modules/exif-reader": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/exif-reader/-/exif-reader-2.0.3.tgz", + "integrity": "sha512-zFbQvguwT9JkqyYhR7pjE1Yn8SagwaGLNRU0Oh14xFa1paSf5Gzxn4gxgk0XhnudI0UIqU+HgnBX93+nva592A==", "license": "MIT" }, "node_modules/exiftool-vendored": { From b2248b38603a4246db6d887c716212a55f9a819b Mon Sep 17 00:00:00 2001 From: Benjy Date: Sat, 26 Sep 2026 20:21:08 +0200 Subject: [PATCH 2/2] Keep the folder size index current from the write, not from the sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The index has three ways of learning a folder's size: a first pass, a watcher, and an adaptive sweep that walks folders again looking for changes. Nothing told it about a write the application itself had just made — and the application knows the exact number of bytes at that moment, which is the one case where no traversal is needed at all. So after an upload, a save in the editor, a new folder or a document created from a template, the size shown for the folder was the one from before, until a sweep happened to come round to it. On a large volume that is minutes. `services/folderSizeHooks.js` takes the delta each operation already has and propagates it to the folder and its ancestors. Four call sites here: the upload, the editor save, a new folder, and a document created with contents. Every hook is best-effort — gated on the feature being on, and swallowing its own errors — because a folder size is not worth failing a write for. The editor save reads what the file weighed before it wrote, so the folder gains the difference rather than the whole of the new file. That is the whole reason the hook takes two numbers. `folderSizeIndex` also records which mode the index was built in, and `folderSizeManager` measures again when it starts in a different one: sizes measured as "apparent" and sizes measured on disk are not the same numbers, and keeping the other mode's answers was showing a total nobody could reconcile with anything. ## Checks 81 tests across `folder-size-hooks`, `folder-size-flush`, `folder-size-disabled`, `folder-size-mode-change`, `folderSizeIndexer` and the `folderSize` route. Making `onFileWritten` say nothing — the behaviour being replaced — turns 34 of them red. Whole backend suite: 2 545 passed, 2 failed, the two that fail on `main` on its own. `frontend/.eslintrc.cjs` gains `env: { browser, es2022 }`. `ecmaVersion: 'latest'` sets what syntax is allowed and not what exists at run time, so `globalThis` — standard since ES2020 — read as an undefined name in nine places. That is nine errors fewer in `npm run lint`, and it would have applied to any file that reached for it. The hooks belong in three more places — the two office editors and the archive extraction — and those call sites travel with their own batches. --- backend/src/routes/editor.js | 21 +- backend/src/routes/files/file.js | 2 + backend/src/routes/files/folder.js | 3 + backend/src/routes/folderSize.js | 11 +- backend/src/routes/upload.js | 5 + backend/src/services/folderSizeHooks.js | 395 ++++++++++++++++++ backend/src/services/folderSizeIndex.js | 21 + backend/src/services/folderSizeIndexer.js | 7 - backend/src/services/folderSizeManager.js | 27 +- backend/tests/routes/folderSize.test.js | 4 +- .../services/folder-size-disabled.test.js | 4 + .../tests/services/folder-size-flush.test.js | 2 +- .../tests/services/folder-size-hooks.test.js | 280 +++++++++++++ .../services/folder-size-mode-change.test.js | 103 +++++ .../tests/services/folderSizeIndexer.test.js | 15 +- frontend/.eslintrc.cjs | 7 + frontend/src/stores/folderSize.js | 18 +- 17 files changed, 880 insertions(+), 45 deletions(-) create mode 100644 backend/src/services/folderSizeHooks.js create mode 100644 backend/tests/services/folder-size-hooks.test.js create mode 100644 backend/tests/services/folder-size-mode-change.test.js diff --git a/backend/src/routes/editor.js b/backend/src/routes/editor.js index 946fe26ce..b49eb1b89 100644 --- a/backend/src/routes/editor.js +++ b/backend/src/routes/editor.js @@ -6,6 +6,7 @@ const { normalizeRelativePath } = require('../utils/pathUtils'); const { ensureDir } = require('../utils/fsUtils'); const { ACTIONS, authorizeAndResolve } = require('../services/authorizationService'); const versions = require('../services/versions/operations'); +const folderSizeHooks = require('../services/folderSizeHooks'); const asyncHandler = require('../utils/asyncHandler'); const { sendTextFile } = require('../utils/textFileResponse'); const { ValidationError, ForbiddenError, NotFoundError } = require('../errors/AppError'); @@ -133,10 +134,11 @@ router.put( const { absolutePath } = resolved; await ensureDir(path.dirname(absolutePath)); - const existed = await fs - .stat(absolutePath) - .then((stats) => stats.isFile()) - .catch(() => false); + // What the file weighed before, for the index: a save replaces content, so the + // folder it sits in gains the difference rather than the whole of the new file. + const before = await fs.stat(absolutePath).catch(() => null); + const existed = Boolean(before?.isFile()); + const previousSize = existed ? before.size : 0; // Written back in the encoding it already had: a UTF-16 file saved as UTF-8 // reads perfectly well here and breaks whatever wrote it. @@ -165,6 +167,17 @@ router.put( explicit: true, } ); + // The index takes the difference the save made, from the size it can already + // see, instead of waiting for the periodic sweep to walk the folder again. + const updated = await fs.stat(absolutePath).catch(() => null); + if (updated) { + if (existed) { + await folderSizeHooks.onFileReplaced(absolutePath, previousSize, updated.size); + } else { + await folderSizeHooks.onFileWritten(absolutePath, updated.size); + } + } + res.send({ success: true }); }) ); diff --git a/backend/src/routes/files/file.js b/backend/src/routes/files/file.js index 31bc4f22b..21f6b74d1 100644 --- a/backend/src/routes/files/file.js +++ b/backend/src/routes/files/file.js @@ -3,6 +3,7 @@ const fs = require('fs/promises'); const { normalizeRelativePath, ensureValidName, splitName } = require('../../utils/pathUtils'); const { ACTIONS, authorizeAndResolve } = require('../../services/authorizationService'); +const folderSizeHooks = require('../../services/folderSizeHooks'); const asyncHandler = require('../../utils/asyncHandler'); const { ValidationError, ForbiddenError, NotFoundError } = require('../../errors/AppError'); const { buildItemMetadata } = require('./utils'); @@ -184,6 +185,7 @@ router.post( baseName, contents ); + if (contents) await folderSizeHooks.onFileWritten(absolutePath, contents.length); const item = await buildItemMetadata(absolutePath, parentRelative, finalName); res.status(201).json({ success: true, item }); diff --git a/backend/src/routes/files/folder.js b/backend/src/routes/files/folder.js index 5686c93bc..86981ed40 100644 --- a/backend/src/routes/files/folder.js +++ b/backend/src/routes/files/folder.js @@ -2,6 +2,7 @@ const fs = require('fs/promises'); const { normalizeRelativePath, ensureValidName } = require('../../utils/pathUtils'); const { reserveAvailableName } = require('../../utils/placeWithoutOverwrite'); const { ACTIONS, authorizeAndResolve } = require('../../services/authorizationService'); +const folderSizeHooks = require('../../services/folderSizeHooks'); const asyncHandler = require('../../utils/asyncHandler'); const { ValidationError, ForbiddenError, NotFoundError } = require('../../errors/AppError'); const { buildItemMetadata } = require('./utils'); @@ -64,6 +65,8 @@ router.post( { isDirectory: true, style: 'folder' } ); + folderSizeHooks.onFolderCreated(folderAbsolute); + const item = await buildItemMetadata(folderAbsolute, parentRelative, finalName); res.status(201).json({ success: true, item }); }) diff --git a/backend/src/routes/folderSize.js b/backend/src/routes/folderSize.js index df7a9604f..4eb3db286 100644 --- a/backend/src/routes/folderSize.js +++ b/backend/src/routes/folderSize.js @@ -2,7 +2,7 @@ const express = require('express'); const fs = require('fs/promises'); const { normalizeRelativePath, parsePathSpace, resolveVolumePath } = require('../utils/pathUtils'); const { resolvePathWithAccess } = require('../services/accessManager'); -const { getDb } = require('../services/db'); +const { getIndexDb } = require('../services/indexDb'); const folderSizeIndex = require('../services/folderSizeIndex'); const { getVolumeScope } = require('../services/folderSizeIndexer'); const folderSizeManager = require('../services/folderSizeManager'); @@ -69,9 +69,11 @@ const lookupFolderSize = async (context, inputRelRaw) => { // resolution so size is available even when navigation is denied. const absolutePath = resolved?.absolutePath ?? (await fallbackAbsolutePath(context, inputRel)); - const db = await getDb(); + const db = await getIndexDb(); const scope = getVolumeScope(); - const withinRoot = Boolean(absolutePath && folderSizeIndex.isWithinRoot(scope.root, absolutePath)); + const withinRoot = Boolean( + absolutePath && folderSizeIndex.isWithinRoot(scope.root, absolutePath) + ); const excluded = withinRoot && folderSizeExclusions.isExcluded(absolutePath, scope); const entry = withinRoot ? folderSizeIndex.getByAbsolutePath(db, absolutePath) : null; @@ -191,7 +193,7 @@ router.post( } queueRefreshDirectory(absolutePath); - const db = await getDb(); + const db = await getIndexDb(); res.status(202).json({ ...indexResult(db, scope, absolutePath, resolved.relativePath, true), refreshPending: true, @@ -228,7 +230,6 @@ router.post( const touchPaths = []; for (const p of limited) { try { - // eslint-disable-next-line no-await-in-loop const { result, absolutePath } = await lookupFolderSize( context, typeof p === 'string' ? p : '' diff --git a/backend/src/routes/upload.js b/backend/src/routes/upload.js index 8b801294b..6071324a6 100644 --- a/backend/src/routes/upload.js +++ b/backend/src/routes/upload.js @@ -12,6 +12,7 @@ const activityLog = require('../services/activityLog'); const { normalizeRelativePath } = require('../utils/pathUtils'); const { ACTIONS, authorizeAndResolve } = require('../services/authorizationService'); const logger = require('../utils/logger'); +const folderSizeHooks = require('../services/folderSizeHooks'); const asyncHandler = require('../utils/asyncHandler'); const { ForbiddenError, ValidationError } = require('../errors/AppError'); @@ -100,6 +101,10 @@ router.post( for (const file of req.files.filedata) { const stats = await fs.stat(file.path); + // The file's exact size is already known here: the index takes a precise + // positive delta, with no filesystem traversal of its own. + folderSizeHooks.onFileWritten(file.path, stats.size); + // Prefer logicalPath set by upload service; fall back to empty string const logicalPath = normalizeRelativePath(file.logicalPath || ''); const parentPath = normalizeRelativePath(path.dirname(logicalPath)); diff --git a/backend/src/services/folderSizeHooks.js b/backend/src/services/folderSizeHooks.js new file mode 100644 index 000000000..53e5f2802 --- /dev/null +++ b/backend/src/services/folderSizeHooks.js @@ -0,0 +1,395 @@ +/** + * Folder size index — write hooks. + * + * NextExplorer's own write operations (upload, delete, move, copy, folder + * creation) already know the exact byte impact of files, so they update the + * folder_size_index directly with a precise, ancestor-propagating delta — no + * extra filesystem traversal, no synchronous `du`. Directory transfers are + * intentionally different: an authoritative subtree scan runs after the whole + * operation completes, rather than trusting stale copied index metadata. + * external writes (other Samba/NFS clients) are picked up separately by the + * watcher and the periodic reconciliation. + * + * Every hook is best-effort: it is gated on the feature being enabled, resolves + * the shared main-thread database connection, and swallows/logs any error so an + * indexing hiccup can never fail (or slow down materially) the user's actual + * file operation. All writes are tiny synchronous SQLite transactions. + */ +const path = require('path'); + +const config = require('../config/index'); +const logger = require('../utils/logger'); +const { getIndexDb } = require('./indexDb'); +const folderSizeIndex = require('./folderSizeIndex'); +const { getVolumeScope } = require('./folderSizeIndexer'); +const folderSizeManager = require('./folderSizeManager'); +const transferState = require('./folderSizeTransferState'); +const exclusions = require('./folderSizeExclusions'); + +const isEnabled = () => config.folderSize.enabled; + +const withIndex = async (fn) => { + if (!isEnabled()) return; + try { + const db = await getIndexDb(); + const scope = getVolumeScope(); + return await fn(db, scope); + } catch (err) { + logger.debug({ err, component: 'folderSizeIndexer' }, 'Folder size hook failed (non-fatal)'); + } +}; + +/** + * Tell the search index what changed, if it is running. + * + * These hooks are where the application says "something on disk moved", which + * is what both indexes need to hear — so the notification lives here rather + * than being repeated at every one of the dozen call sites, where one would + * eventually be forgotten. Required late and never awaited: a search index + * that is off, busy or broken must not slow a write down or fail it. + */ +const notifySearchIndex = (method, ...args) => { + try { + const searchIndexManager = require('./searchIndexManager'); + Promise.resolve(searchIndexManager[method](...args)).catch(() => {}); + } catch { + // The search index is optional; a write is not. + } +}; + +/** A file has been created/written at `absolutePath` with `size` bytes. */ +const onFileWritten = (absolutePath, size) => { + notifySearchIndex('onFileChanged', absolutePath); + return withIndex((db, scope) => { + if (exclusions.isExcluded(absolutePath, scope)) return; + folderSizeIndex.applyDelta(db, scope, path.dirname(absolutePath), Number(size) || 0, { + entryDelta: 1, + }); + }); +}; + +/** A file was replaced in place; only its byte delta changes the index. */ +const onFileReplaced = (absolutePath, previousSize, size) => { + notifySearchIndex('onFileChanged', absolutePath); + return withIndex((db, scope) => { + if (exclusions.isExcluded(absolutePath, scope)) return; + folderSizeIndex.applyDelta( + db, + scope, + path.dirname(absolutePath), + (Number(size) || 0) - (Number(previousSize) || 0) + ); + }); +}; + +/** An empty folder has been created at `absolutePath`. */ +const onFolderCreated = (absolutePath) => { + notifySearchIndex('onFolderAdded', absolutePath); + return withIndex((db, scope) => { + if (exclusions.isExcluded(absolutePath, scope)) return; + folderSizeIndex.upsertScanEntry(db, scope, { + absolutePath, + sizeBytes: 0, + entryCount: 0, + lastFullScanAt: new Date().toISOString(), + }); + folderSizeIndex.applyDelta(db, scope, path.dirname(absolutePath), 0, { entryDelta: 1 }); + }); +}; + +/** + * A complete directory tree was created by an application operation. The + * manager scans this tree alone and applies one precise delta to its parent. + */ +const onDirectoryTreeCreated = (absolutePath) => { + notifySearchIndex('onTreeAdded', absolutePath); + if (!isEnabled()) return null; + const refresh = folderSizeManager.refreshSubtree(absolutePath); + Promise.resolve(refresh).catch((err) => { + logger.debug( + { err, component: 'folderSizeIndexer' }, + 'Folder subtree index refresh failed (non-fatal)' + ); + }); + return refresh; +}; + +/** + * An entry has been deleted. For a file the size is known; for a directory the + * recursive size comes from the index (its subtree is dropped and removed from + * the ancestors). + */ +const onEntryDeleted = (absolutePath, { isDirectory, size } = {}) => { + notifySearchIndex('onPathRemoved', absolutePath); + folderSizeManager.invalidateSubtree(absolutePath, 'entry-deleted'); + return withIndex((db, scope) => { + if (exclusions.isExcluded(absolutePath, scope)) return; + if (isDirectory) { + const removed = folderSizeIndex.getByAbsolutePath(db, absolutePath); + const removedSize = removed ? removed.sizeBytes : 0; + folderSizeIndex.removeSubtree(db, scope, absolutePath); + if (folderSizeIndex.isWithinRoot(scope.root, absolutePath) && absolutePath !== scope.root) { + folderSizeIndex.applyDelta(db, scope, path.dirname(absolutePath), -removedSize, { + entryDelta: -1, + }); + } + } else { + folderSizeIndex.applyDelta(db, scope, path.dirname(absolutePath), -(Number(size) || 0), { + entryDelta: -1, + }); + } + }); +}; + +const sizeOfMoved = (db, scope, sourceAbsolutePath, { isDirectory, size }) => { + if (!isDirectory) return Number(size) || 0; + const entry = folderSizeIndex.getByAbsolutePath(db, sourceAbsolutePath); + return entry ? entry.sizeBytes : 0; +}; + +/** + * Mark a directory target before rsync starts writing it. A pending index row + * makes the UI show an intentional placeholder and blocks all background scans + * of that tree until the operation has finished. + */ +const beginDirectoryTransfer = async (targetAbsolutePath) => { + if (!isEnabled()) return; + folderSizeManager.invalidateSubtree(targetAbsolutePath, 'directory-transfer-started'); + transferState.begin(targetAbsolutePath); + await withIndex((db, scope) => { + if (exclusions.isExcluded(targetAbsolutePath, scope)) return; + if (!folderSizeIndex.isWithinRoot(scope.root, targetAbsolutePath)) return; + + const previous = folderSizeIndex.getByAbsolutePath(db, targetAbsolutePath); + if (previous) { + folderSizeIndex.removeSubtree(db, scope, targetAbsolutePath); + folderSizeIndex.applyDelta(db, scope, path.dirname(targetAbsolutePath), -previous.sizeBytes, { + entryDelta: -1, + }); + } + + folderSizeIndex.upsertPendingDirectoryEntry(db, scope, { + absolutePath: targetAbsolutePath, + sizeBytes: 0, + entryCount: 0, + }); + folderSizeIndex.applyDelta(db, scope, path.dirname(targetAbsolutePath), 0, { + entryDelta: 1, + }); + }); +}; + +/** + * Keep scans away from a folder being filled under a hidden name, until it is + * put in place. + * + * A copy fills a hidden `.nextexplorer-copying-` folder beside where it goes, + * and moves it under its name once whole. The size walker counts hidden entries + * like any other, so a scan of the destination meanwhile would publish a size + * with half a copy in it, and give the hidden folder a row of its own that + * outlives the move. Only the lock is taken: no index row is made for a name + * nobody sees. The row is made under the name the folder lands at + * (beginDirectoryTransfer), and `release()` lets scans back in, however the + * copy ended. + */ +const holdHiddenDirectory = (absolutePath) => { + if (!isEnabled()) return { release: () => {} }; + transferState.begin(absolutePath); + let released = false; + return { + release: () => { + if (released) return; + released = true; + transferState.finish(absolutePath); + }, + }; +}; + +/** An entry has been moved from `sourceAbsolutePath` to `targetAbsolutePath`. */ +const onEntryMoved = (sourceAbsolutePath, targetAbsolutePath, meta = {}) => { + notifySearchIndex('onPathMoved', sourceAbsolutePath, targetAbsolutePath); + folderSizeManager.invalidateSubtree(sourceAbsolutePath, 'entry-moved'); + return withIndex((db, scope) => { + const sourceExcluded = exclusions.isExcluded(sourceAbsolutePath, scope); + const targetExcluded = exclusions.isExcluded(targetAbsolutePath, scope); + if (sourceExcluded && targetExcluded) return; + if (sourceExcluded && !targetExcluded) { + if (meta.isDirectory) { + folderSizeIndex.upsertPendingDirectoryEntry(db, scope, { + absolutePath: targetAbsolutePath, + sizeBytes: 0, + entryCount: 0, + }); + folderSizeIndex.applyDelta(db, scope, path.dirname(targetAbsolutePath), 0, { + entryDelta: 1, + }); + } else { + folderSizeIndex.applyDelta( + db, + scope, + path.dirname(targetAbsolutePath), + Number(meta.size) || 0, + { + entryDelta: 1, + } + ); + } + return; + } + if (!sourceExcluded && targetExcluded) { + const bytes = sizeOfMoved(db, scope, sourceAbsolutePath, meta); + folderSizeIndex.applyDelta(db, scope, path.dirname(sourceAbsolutePath), -bytes, { + entryDelta: -1, + }); + if (meta.isDirectory) folderSizeIndex.removeSubtree(db, scope, sourceAbsolutePath); + return; + } + const bytes = sizeOfMoved(db, scope, sourceAbsolutePath, meta); + folderSizeIndex.applyDelta(db, scope, path.dirname(sourceAbsolutePath), -bytes, { + entryDelta: -1, + }); + if (meta.isDirectory) { + // A cached subtree can be incomplete or out of date when the directory + // originated outside NextExplorer. Drop it and let the post-operation + // authoritative scan rebuild the destination instead of moving bad data. + folderSizeIndex.removeSubtree(db, scope, sourceAbsolutePath); + if ( + !meta.directoryTransferPrepared && + folderSizeIndex.isWithinRoot(scope.root, targetAbsolutePath) + ) { + folderSizeIndex.upsertPendingDirectoryEntry(db, scope, { + absolutePath: targetAbsolutePath, + sizeBytes: 0, + entryCount: 0, + }); + folderSizeIndex.applyDelta(db, scope, path.dirname(targetAbsolutePath), 0, { + entryDelta: 1, + }); + } + return; + } + folderSizeIndex.applyDelta(db, scope, path.dirname(targetAbsolutePath), bytes, { + entryDelta: 1, + }); + }); +}; + +/** + * An entry has been copied to `targetAbsolutePath`. The destination gains the + * copied bytes. A copied directory is marked pending so a single authoritative + * scan can rebuild it once the complete transfer has finished. Cloning the + * source index is deliberately avoided: source rows may be stale or incomplete. + */ +const onEntryCopied = async (targetAbsolutePath, meta = {}) => { + return withIndex((db, scope) => { + if (exclusions.isExcluded(targetAbsolutePath, scope)) return; + if (meta.isDirectory) { + if (meta.directoryTransferPrepared) return; + if (!folderSizeIndex.isWithinRoot(scope.root, targetAbsolutePath)) return; + folderSizeIndex.upsertPendingDirectoryEntry(db, scope, { + absolutePath: targetAbsolutePath, + sizeBytes: 0, + entryCount: 0, + }); + folderSizeIndex.applyDelta(db, scope, path.dirname(targetAbsolutePath), 0, { + entryDelta: 1, + }); + return; + } + + folderSizeIndex.applyDelta( + db, + scope, + path.dirname(targetAbsolutePath), + Number(meta.size) || 0, + { + entryDelta: 1, + } + ); + }); +}; + +/** + * Queue authoritative scans once a transfer has settled. Keep each directory + * protected until that scan has committed its root aggregate: otherwise an + * on-view refresh can observe only part of the subtree and publish a zero or + * partial size over the pending entry. + */ +const refreshTransferredDirectories = (absolutePaths = []) => { + if (!isEnabled()) return; + const uniquePaths = [...new Set(absolutePaths.filter(Boolean))]; + for (const absolutePath of uniquePaths) { + (async () => { + try { + const result = await folderSizeManager.refreshSubtree(absolutePath, { + allowActiveTransfer: true, + }); + if (!result) { + logger.warn( + { component: 'folderSizeIndexer', path: absolutePath }, + 'Transferred directory refresh did not start' + ); + } + } catch (err) { + logger.debug( + { err, component: 'folderSizeIndexer', path: absolutePath }, + 'Transferred directory refresh failed (non-fatal)' + ); + } finally { + transferState.finish(absolutePath); + } + })(); + } +}; + +/** + * An entry has been renamed in place (same parent directory). Bytes and parent + * are unchanged, so there is no delta — but an indexed directory's subtree keys + * must follow the new name. Files are not individually indexed, so this is a + * no-op for them. + */ +const onEntryRenamed = (sourceAbsolutePath, targetAbsolutePath) => { + notifySearchIndex('onPathMoved', sourceAbsolutePath, targetAbsolutePath); + folderSizeManager.invalidateSubtree(sourceAbsolutePath, 'entry-renamed'); + return withIndex((db, scope) => { + const sourceExcluded = exclusions.isExcluded(sourceAbsolutePath, scope); + const targetExcluded = exclusions.isExcluded(targetAbsolutePath, scope); + if (sourceExcluded && targetExcluded) return; + if (sourceExcluded || targetExcluded) { + if (!sourceExcluded) { + const entry = folderSizeIndex.getByAbsolutePath(db, sourceAbsolutePath); + const removedSize = entry?.sizeBytes || 0; + folderSizeIndex.removeSubtree(db, scope, sourceAbsolutePath); + folderSizeIndex.applyDelta(db, scope, path.dirname(sourceAbsolutePath), -removedSize, { + entryDelta: -1, + }); + } else if (!targetExcluded) { + folderSizeIndex.upsertPendingDirectoryEntry(db, scope, { + absolutePath: targetAbsolutePath, + sizeBytes: 0, + entryCount: 0, + }); + folderSizeIndex.applyDelta(db, scope, path.dirname(targetAbsolutePath), 0, { + entryDelta: 1, + }); + } + return; + } + const entry = folderSizeIndex.getByAbsolutePath(db, sourceAbsolutePath); + if (entry) folderSizeIndex.reparentSubtree(db, scope, sourceAbsolutePath, targetAbsolutePath); + }); +}; + +module.exports = { + onFileWritten, + onFileReplaced, + onFolderCreated, + onDirectoryTreeCreated, + onEntryDeleted, + beginDirectoryTransfer, + holdHiddenDirectory, + onEntryMoved, + onEntryCopied, + refreshTransferredDirectories, + onEntryRenamed, +}; diff --git a/backend/src/services/folderSizeIndex.js b/backend/src/services/folderSizeIndex.js index 734c16013..1612f81ad 100644 --- a/backend/src/services/folderSizeIndex.js +++ b/backend/src/services/folderSizeIndex.js @@ -168,6 +168,25 @@ const setIndexVersion = (db, scope, version = CURRENT_INDEX_VERSION) => { ); }; +const indexModeKey = (scope) => `folder_size_index_mode:${scope.label}:${pathHash(scope.root)}`; + +/** + * The mode the sizes on record were measured in, or null before this was kept. + * + * `shallow` counts a folder's own entries and `full` everything under it, so a + * size measured one way is simply wrong read the other. Nothing recorded which + * one had been used: changing FOLDER_SIZE_MODE and restarting kept every size + * from before, and the baseline skipped itself because the volume was already + * indexed. Now that Settings can change it without a restart, it has to be + * written down. + */ +const getIndexMode = (db, scope) => + prep(db, 'SELECT value FROM meta WHERE key = ?').pluck().get(indexModeKey(scope)) || null; + +const setIndexMode = (db, scope, mode) => { + prep(db, 'INSERT OR REPLACE INTO meta(key, value) VALUES (?, ?)').run(indexModeKey(scope), mode); +}; + /** * Apply an incremental byte delta to a folder and propagate it up every * ancestor to the volume root, in a single transaction. Rows that do not yet @@ -506,6 +525,8 @@ module.exports = { countByVolume, getIndexVersion, setIndexVersion, + getIndexMode, + setIndexMode, applyDelta, upsertScanEntry, bulkUpsertScanEntries, diff --git a/backend/src/services/folderSizeIndexer.js b/backend/src/services/folderSizeIndexer.js index e8f8d09a2..f2debccd7 100644 --- a/backend/src/services/folderSizeIndexer.js +++ b/backend/src/services/folderSizeIndexer.js @@ -332,7 +332,6 @@ const scanTree = async (db, scope, rootAbs, options = {}) => { let entries = null; try { reportProgress('readdir', frame.abs); - // eslint-disable-next-line no-await-in-loop entries = await limit(() => guardedFs('readdir', frame.abs, () => fs.readdir(frame.abs, { withFileTypes: true })) ); @@ -367,7 +366,6 @@ const scanTree = async (db, scope, rootAbs, options = {}) => { throwIfAborted(); const paths = filePaths.slice(offset, offset + batchSize); reportProgress('stat', frame.abs); - // eslint-disable-next-line no-await-in-loop const fileSizes = await Promise.all( paths.map((filePath) => limit(async () => { @@ -384,7 +382,6 @@ const scanTree = async (db, scope, rootAbs, options = {}) => { frame.directFileBytes += fileSizes.reduce((total, size) => total + size, 0); files += paths.length; if (offset + paths.length < filePaths.length) { - // eslint-disable-next-line no-await-in-loop await yieldAndPause(); } } @@ -416,7 +413,6 @@ const scanTree = async (db, scope, rootAbs, options = {}) => { } if (folders % yieldEvery === 0) { - // eslint-disable-next-line no-await-in-loop await yieldAndPause(); } } @@ -508,7 +504,6 @@ const aggregateDirectory = async (db, scope, absDir, options = {}) => { } else if (entry.isFile()) { entryCount += 1; try { - // eslint-disable-next-line no-await-in-loop directFileBytes += ( await withIoTimeout('stat', full, () => fs.stat(full), { timeoutMs: ioTimeoutMs }) ).size; @@ -680,7 +675,6 @@ const reconcile = async (db, scope, options = {}) => { for (const row of rows) { if (signal?.aborted || (maxDirectories > 0 && processed >= maxDirectories)) break; processed += 1; - // eslint-disable-next-line no-await-in-loop await handleRow(row); cursor = row.relativePath; } @@ -689,7 +683,6 @@ const reconcile = async (db, scope, options = {}) => { exhausted = true; break; } - // eslint-disable-next-line no-await-in-loop if (pauseMs > 0) await sleep(pauseMs); } diff --git a/backend/src/services/folderSizeManager.js b/backend/src/services/folderSizeManager.js index c4cee4484..3b8f84d74 100644 --- a/backend/src/services/folderSizeManager.js +++ b/backend/src/services/folderSizeManager.js @@ -30,6 +30,7 @@ const folderSizeIndex = require('./folderSizeIndex'); const transferState = require('./folderSizeTransferState'); const exclusions = require('./folderSizeExclusions'); const { getDb } = require('./db'); +const { getIndexDb } = require('./indexDb'); let db = null; let scope = null; @@ -138,7 +139,6 @@ const flush = async () => { for (const abs of dirs) { let agg; try { - // eslint-disable-next-line no-await-in-loop agg = await indexer.aggregateDirectory(db, scope, abs, { mode: config.folderSize.mode, shouldExclude: (absDir) => exclusions.isExcluded(absDir, scope), @@ -191,7 +191,6 @@ const touch = async (absDirs = []) => { if (transferState.isRelatedToActiveTransfer(abs)) continue; let stat; try { - // eslint-disable-next-line no-await-in-loop stat = await indexer.withIoTimeout('stat', abs, () => fsp.stat(abs)); } catch (err) { if (isFolderSizeIoSafetyError(err)) { @@ -282,8 +281,17 @@ const baselineIfNeeded = async () => { const existing = folderSizeIndex.countByVolume(db, scope.label); const storedVersion = folderSizeIndex.getIndexVersion(db, scope); const needsVersionUpgrade = existing > 0 && storedVersion < folderSizeIndex.CURRENT_INDEX_VERSION; - const rebuild = config.folderSize.rebuild || needsVersionUpgrade; + // Sizes measured in the other mode are wrong rather than stale, so they go. + // An index from before the mode was recorded is taken to be in the current + // one, as it always was: throwing a working index away on upgrade to learn + // something already true would cost a whole walk for nothing. + const storedMode = folderSizeIndex.getIndexMode(db, scope); + const modeChanged = existing > 0 && storedMode !== null && storedMode !== config.folderSize.mode; + const rebuild = config.folderSize.rebuild || needsVersionUpgrade || modeChanged; if (existing > 0 && !rebuild) { + // Recorded here too, so an index built before the mode was kept learns it + // on the first start that has nothing to rebuild. + if (storedMode === null) folderSizeIndex.setIndexMode(db, scope, config.folderSize.mode); log('info', 'Baseline skipped (volume already indexed)', { folders: existing, indexVersion: storedVersion, @@ -294,7 +302,12 @@ const baselineIfNeeded = async () => { folderSizeIndex.removeSubtree(db, scope, scope.root); log('info', 'Rebuild requested — cleared existing index', { folders: existing, - reason: config.folderSize.rebuild ? 'manual' : 'index-version-upgrade', + reason: config.folderSize.rebuild + ? 'manual' + : modeChanged + ? 'mode-changed' + : 'index-version-upgrade', + ...(modeChanged ? { fromMode: storedMode, toMode: config.folderSize.mode } : {}), fromVersion: storedVersion, toVersion: folderSizeIndex.CURRENT_INDEX_VERSION, }); @@ -306,6 +319,7 @@ const baselineIfNeeded = async () => { shouldExclude: (absDir) => exclusions.isExcluded(absDir, scope), }); folderSizeIndex.setIndexVersion(db, scope); + folderSizeIndex.setIndexMode(db, scope, config.folderSize.mode); log('info', 'Baseline walk complete', { ...result, ms: Date.now() - started }); }; @@ -321,9 +335,10 @@ const pruneExcludedIndexEntries = (relativePaths = exclusions.effectivePaths()) }; const init = async () => { - db = await getDb(); + db = await getIndexDb(); scope = indexer.getVolumeScope(); - exclusions.loadFromDatabase(db); + // The folders not to measure are a setting, and settings stay in app.db. + exclusions.loadFromDatabase(await getDb()); pruneExcludedIndexEntries(); // Baseline once (or on explicit rebuild). Async + cooperative yields, so it diff --git a/backend/tests/routes/folderSize.test.js b/backend/tests/routes/folderSize.test.js index 7bc44946a..9db5d8d99 100644 --- a/backend/tests/routes/folderSize.test.js +++ b/backend/tests/routes/folderSize.test.js @@ -29,13 +29,13 @@ const buildContext = async ({ user, userVolumes = false, folderSizeMode = 'full' }, }); - const { getDb } = env.requireFresh('src/services/db'); + const { getIndexDb } = env.requireFresh('src/services/indexDb'); const folderSizeIndex = env.requireFresh('src/services/folderSizeIndex'); const indexer = env.requireFresh('src/services/folderSizeIndexer'); const manager = env.requireFresh('src/services/folderSizeManager'); const routes = env.requireFresh('src/routes/folderSize'); - const db = await getDb(); + const db = await getIndexDb(); const scope = { root: env.volumeDir, label: 'volume' }; const app = createTestApp({ router: routes, mountPath: '/api', user }); diff --git a/backend/tests/services/folder-size-disabled.test.js b/backend/tests/services/folder-size-disabled.test.js index 417ef20fa..2352a3419 100644 --- a/backend/tests/services/folder-size-disabled.test.js +++ b/backend/tests/services/folder-size-disabled.test.js @@ -5,6 +5,7 @@ const MODULES = [ 'src/config/env', 'src/config/index', 'src/services/folderSizeManager', + 'src/services/folderSizeHooks', 'src/services/folderSizeTransferState', ]; @@ -30,11 +31,14 @@ describe('Folder size disabled mode', () => { const config = env.requireFresh('src/config/index'); const manager = env.requireFresh('src/services/folderSizeManager'); + const hooks = env.requireFresh('src/services/folderSizeHooks'); const transferState = env.requireFresh('src/services/folderSizeTransferState'); expect(config.folderSize).toMatchObject({ mode: 'off', enabled: false }); await manager.start(); + await hooks.beginDirectoryTransfer('/tmp/folder-size-disabled-transfer'); + hooks.refreshTransferredDirectories(['/tmp/folder-size-disabled-transfer']); expect(transferState.isRelatedToActiveTransfer('/tmp/folder-size-disabled-transfer')).toBe( false diff --git a/backend/tests/services/folder-size-flush.test.js b/backend/tests/services/folder-size-flush.test.js index dc3bb7223..3036773ff 100644 --- a/backend/tests/services/folder-size-flush.test.js +++ b/backend/tests/services/folder-size-flush.test.js @@ -45,7 +45,7 @@ const setup = async (extraEnv = {}) => { const transferState = currentEnv.requireFresh('src/services/folderSizeTransferState'); const folderSizeIndex = currentEnv.requireFresh('src/services/folderSizeIndex'); manager = currentEnv.requireFresh('src/services/folderSizeManager'); - const db = await currentEnv.requireFresh('src/services/db').getDb(); + const db = await currentEnv.requireFresh('src/services/indexDb').getIndexDb(); await manager.start(); diff --git a/backend/tests/services/folder-size-hooks.test.js b/backend/tests/services/folder-size-hooks.test.js new file mode 100644 index 000000000..fe9749d53 --- /dev/null +++ b/backend/tests/services/folder-size-hooks.test.js @@ -0,0 +1,280 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import path from 'node:path'; +import fs from 'node:fs/promises'; + +import { setupTestEnv } from '../helpers/env-test-utils.js'; + +/** + * The folder-size index hooks, and the promise made in their own header: + * "an indexing hiccup can never fail (or slow down materially) the user's + * actual file operation". + * + * That promise is the design. Every hook runs inside an upload, a delete, a + * move — operations that must succeed whether or not a size index is healthy, + * enabled, or present at all. A throw escaping any of them turns bookkeeping + * into a failed file operation, and a size index is exactly the thing that is + * broken on somebody's NAS at 3am. + * + * Run against the real index and a real database rather than mocks, because the + * first attempt at this file mocked the config module, the mock silently did not + * apply, the feature was off for every test, and thirty-four assertions passed + * against a no-op. The switch is asserted before anything else for that reason. + */ + +let ctx; + +const setup = async ({ mode = 'full', exclude } = {}) => { + const env = await setupTestEnv({ + tag: 'folder-size-hooks-', + modules: [ + 'src/config/env', + 'src/config/index', + 'src/services/db', + 'src/services/folderSizeIndex', + 'src/services/folderSizeIndexer', + 'src/services/folderSizeManager', + 'src/services/folderSizeHooks', + ], + env: { FOLDER_SIZE_MODE: mode, ...(exclude ? { FOLDER_SIZE_EXCLUDE_PATHS: exclude } : {}) }, + }); + const { getIndexDb } = env.requireFresh('src/services/indexDb'); + const index = env.requireFresh('src/services/folderSizeIndex'); + const hooks = env.requireFresh('src/services/folderSizeHooks'); + const db = await getIndexDb(); + ctx = { env, db, index, hooks, volume: env.volumeDir }; + return ctx; +}; + +afterEach(async () => { + if (ctx) { + await ctx.env.cleanup(); + ctx = null; + } +}); + +/** + * A folder restored from the trash or extracted from an archive arrives with + * nothing in the search index. The hook that announces it is the one place to + * tell search, and it must, whether folder sizes are switched on or not. + */ +describe('the search index', () => { + it.each(['off', 'full'])( + 'hears about a whole tree put down, with folder sizes %s', + async (mode) => { + const { env, hooks, volume } = await setup({ mode }); + const manager = env.requireFresh('src/services/searchIndexManager'); + const told = vi.spyOn(manager, 'onTreeAdded').mockResolvedValue(); + + hooks.onDirectoryTreeCreated(path.join(volume, 'Restored')); + + expect(told).toHaveBeenCalledWith(path.join(volume, 'Restored')); + } + ); +}); + +const sizeOf = (absolutePath) => { + const row = ctx.index.getByAbsolutePath(ctx.db, absolutePath); + return row ? row.sizeBytes : null; +}; + +const seedFolder = async (relative) => { + const absolute = path.join(ctx.volume, relative); + await fs.mkdir(absolute, { recursive: true }); + await ctx.hooks.onFolderCreated(absolute); + return absolute; +}; + +/** Every hook, with arguments that make sense for it. */ +const everyHook = (root) => [ + ['onFileWritten', (h) => h.onFileWritten(path.join(root, 'a.txt'), 1024)], + ['onFileReplaced', (h) => h.onFileReplaced(path.join(root, 'a.txt'), 512, 1024)], + ['onFolderCreated', (h) => h.onFolderCreated(path.join(root, 'sub'))], + ['onEntryDeleted (file)', (h) => h.onEntryDeleted(path.join(root, 'a.txt'), { size: 1024 })], + ['onEntryDeleted (dir)', (h) => h.onEntryDeleted(path.join(root, 'sub'), { isDirectory: true })], + ['beginDirectoryTransfer', (h) => h.beginDirectoryTransfer(path.join(root, 'sub'))], + [ + 'onEntryMoved', + (h) => h.onEntryMoved(path.join(root, 'a.txt'), path.join(root, 'b.txt'), { size: 1024 }), + ], + ['onEntryCopied', (h) => h.onEntryCopied(path.join(root, 'b.txt'), { size: 1024 })], + [ + 'refreshTransferredDirectories', + (h) => h.refreshTransferredDirectories([path.join(root, 'sub')]), + ], + ['onEntryRenamed', (h) => h.onEntryRenamed(path.join(root, 'a.txt'), path.join(root, 'c.txt'))], +]; + +describe('the switch these tests depend on', () => { + it('is on, so everything below exercises something', async () => { + const { hooks: h, volume } = await setup(); + const folder = path.join(volume, 'Docs'); + await fs.mkdir(folder, { recursive: true }); + + await h.onFolderCreated(folder); + + expect(sizeOf(folder)).toBe(0); + }); + + it('is off in "off" mode, and then nothing is written', async () => { + const { hooks: h, volume } = await setup({ mode: 'off' }); + const folder = path.join(volume, 'Docs'); + await fs.mkdir(folder, { recursive: true }); + + await h.onFolderCreated(folder); + + expect(sizeOf(folder)).toBeNull(); + }); +}); + +describe('the deltas they apply', () => { + it('adds a written file’s bytes to its parent', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + + await h.onFileWritten(path.join(docs, 'report.txt'), 1024); + + expect(sizeOf(docs)).toBe(1024); + }); + + it('adds up several writes', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + + await h.onFileWritten(path.join(docs, 'a.txt'), 1000); + await h.onFileWritten(path.join(docs, 'b.txt'), 500); + + expect(sizeOf(docs)).toBe(1500); + }); + + /** Replacing changes bytes, and only the difference. */ + it('applies the difference when a file is replaced', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + await h.onFileWritten(path.join(docs, 'a.txt'), 400); + + await h.onFileReplaced(path.join(docs, 'a.txt'), 400, 1000); + + expect(sizeOf(docs)).toBe(1000); + }); + + it('shrinks the parent when the replacement is smaller', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + await h.onFileWritten(path.join(docs, 'a.txt'), 1000); + + await h.onFileReplaced(path.join(docs, 'a.txt'), 1000, 400); + + expect(sizeOf(docs)).toBe(400); + }); + + it('subtracts a deleted file’s bytes', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + await h.onFileWritten(path.join(docs, 'a.txt'), 2048); + + await h.onEntryDeleted(path.join(docs, 'a.txt'), { size: 2048 }); + + expect(sizeOf(docs)).toBe(0); + }); + + /** The propagation is the point: a size is only useful if ancestors know. */ + it('carries a write all the way up the ancestors', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + const year = await seedFolder('Docs/2026'); + + await h.onFileWritten(path.join(year, 'q1.txt'), 4096); + + expect(sizeOf(year)).toBe(4096); + expect(sizeOf(docs)).toBe(4096); + }); + + /** + * A deleted directory's size is not passed in — it comes from the index, + * because only the index knows what its subtree came to. + */ + it('takes a deleted directory’s size from the index and removes it from the parent', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + const year = await seedFolder('Docs/2026'); + await h.onFileWritten(path.join(year, 'q1.txt'), 4096); + + await h.onEntryDeleted(year, { isDirectory: true }); + + expect(sizeOf(docs)).toBe(0); + expect(sizeOf(year)).toBeNull(); + }); + + it('treats a missing size as zero rather than NaN', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + + await h.onFileWritten(path.join(docs, 'a.txt'), undefined); + + expect(sizeOf(docs)).toBe(0); + }); + + it('records a new folder as empty', async () => { + const { hooks: h } = await setup(); + const docs = await seedFolder('Docs'); + + const sub = path.join(docs, 'sub'); + await fs.mkdir(sub); + await h.onFolderCreated(sub); + + expect(sizeOf(sub)).toBe(0); + }); +}); + +describe('paths the operator excluded', () => { + it('are left out of the index entirely', async () => { + const { hooks: h, volume } = await setup({ exclude: 'Stacks' }); + const stacks = path.join(volume, 'Stacks'); + await fs.mkdir(stacks, { recursive: true }); + + await h.onFolderCreated(stacks); + await h.onFileWritten(path.join(stacks, 'huge.img'), 999_999); + + expect(sizeOf(stacks)).toBeNull(); + }); +}); + +describe('nothing they do can fail the operation they run inside', () => { + /** + * The index itself throwing on every call is the shape of a corrupt or + * locked database, which is the case that reaches people and the case no + * other test covers. + */ + it.each(everyHook('/volumes/Docs').map(([name]) => name))( + '%s survives the index throwing', + async (name) => { + const { hooks: h, index, volume } = await setup(); + const docs = await seedFolder('Docs'); + for (const key of Object.keys(index)) { + if (typeof index[key] === 'function') { + vi.spyOn(index, key).mockImplementation(() => { + throw new Error('index corrupt'); + }); + } + } + + const call = everyHook(docs).find(([hookName]) => hookName === name)[1]; + await expect(Promise.resolve(call(h))).resolves.not.toThrow(); + expect(volume).toBeTruthy(); + vi.restoreAllMocks(); + } + ); + + it.each(everyHook('/volumes/Docs').map(([name]) => name))( + '%s does nothing and throws nothing when the feature is off', + async (name) => { + const { hooks: h, volume } = await setup({ mode: 'off' }); + const docs = path.join(volume, 'Docs'); + await fs.mkdir(docs, { recursive: true }); + + const call = everyHook(docs).find(([hookName]) => hookName === name)[1]; + await expect(Promise.resolve(call(h))).resolves.not.toThrow(); + expect(sizeOf(docs)).toBeNull(); + } + ); +}); diff --git a/backend/tests/services/folder-size-mode-change.test.js b/backend/tests/services/folder-size-mode-change.test.js new file mode 100644 index 000000000..2fe8d2052 --- /dev/null +++ b/backend/tests/services/folder-size-mode-change.test.js @@ -0,0 +1,103 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import fs from 'node:fs/promises'; +import path from 'node:path'; +import { setupTestEnv } from '../helpers/env-test-utils.js'; + +/** + * Sizes measured in one mode, read in the other. + * + * `shallow` counts a folder's own entries and `full` everything under it, so a + * size measured one way is not stale in the other, it is wrong. Nothing + * recorded which mode an index was built in, and the baseline skips itself as + * soon as the volume holds any rows — so changing FOLDER_SIZE_MODE and + * restarting went on showing every size from before, for good. + * + * It mattered little while the mode lived in a file nobody edits twice. Now + * that Settings can move it without a restart (#9), the index writes down the + * mode it measured in, and a start in another one measures again. + */ + +const MODULES = [ + 'src/config/env', + 'src/config/index', + 'src/services/indexDb', + 'src/services/folderSizeIndex', + 'src/services/folderSizeIndexer', + 'src/services/folderSizeManager', +]; + +let env = null; + +afterEach(async () => { + if (env) await env.cleanup(); + env = null; +}); + +/** A folder holding one small file directly and one large one further down. */ +const plant = async (volumeDir) => { + const top = path.join(volumeDir, 'Docs', 'Project'); + await fs.mkdir(path.join(top, 'deep'), { recursive: true }); + await fs.writeFile(path.join(top, 'note.txt'), Buffer.alloc(100)); + await fs.writeFile(path.join(top, 'deep', 'video.bin'), Buffer.alloc(50_000)); + return top; +}; + +const sizeOf = async (absolutePath) => { + const { getIndexDb } = env.requireFresh('src/services/indexDb'); + const index = env.requireFresh('src/services/folderSizeIndex'); + return index.getByAbsolutePath(await getIndexDb(), absolutePath)?.sizeBytes ?? null; +}; + +describe('a folder size index built in one mode and started in another', () => { + it('measures again rather than keep the other mode’s sizes', async () => { + env = await setupTestEnv({ + tag: 'folder-size-mode-', + modules: MODULES, + env: { FOLDER_SIZE_MODE: 'full' }, + }); + const project = await plant(env.volumeDir); + + const config = env.requireFresh('src/config/index'); + const manager = env.requireFresh('src/services/folderSizeManager'); + + await manager.start(); + const full = await sizeOf(project); + // Everything under it: the large file two levels down is counted. + expect(full).toBeGreaterThanOrEqual(50_100); + + await manager.stop(); + // What a Settings change does, and what a restart with the other value in + // the environment amounts to. + config.folderSize.mode = 'shallow'; + config.folderSize.enabled = true; + await manager.start(); + + const shallow = await sizeOf(project); + // Its own entries only: the small file, not the large one below. + expect(shallow).toBeLessThan(50_000); + expect(shallow).not.toBe(full); + }); + + it('keeps an index built in the mode it is started in', async () => { + // The rebuild is for a changed mode, not for every start: a walk over a + // large volume is the most expensive thing this worker does. + env = await setupTestEnv({ + tag: 'folder-size-mode-same-', + modules: MODULES, + env: { FOLDER_SIZE_MODE: 'full' }, + }); + const project = await plant(env.volumeDir); + const manager = env.requireFresh('src/services/folderSizeManager'); + + await manager.start(); + const first = await sizeOf(project); + await manager.stop(); + + // A file written behind its back: a new walk would count it, a kept index + // would not until something told it. + await fs.writeFile(path.join(project, 'deep', 'later.bin'), Buffer.alloc(9_000)); + await manager.start(); + + expect(await sizeOf(project)).toBe(first); + }); +}); diff --git a/backend/tests/services/folderSizeIndexer.test.js b/backend/tests/services/folderSizeIndexer.test.js index 7c0f6c0dd..348ade009 100644 --- a/backend/tests/services/folderSizeIndexer.test.js +++ b/backend/tests/services/folderSizeIndexer.test.js @@ -19,10 +19,10 @@ const createContext = async (extraEnv = {}) => { modules: INDEXER_MODULES, env: { FOLDER_SIZE_MODE: 'full', ...extraEnv }, }); - const { getDb } = env.requireFresh('src/services/db'); + const { getIndexDb } = env.requireFresh('src/services/indexDb'); const folderSizeIndex = env.requireFresh('src/services/folderSizeIndex'); const indexer = env.requireFresh('src/services/folderSizeIndexer'); - const db = await getDb(); + const db = await getIndexDb(); const scope = { root: env.volumeDir, label: 'volume' }; return { env, db, folderSizeIndex, indexer, scope }; }; @@ -299,7 +299,6 @@ describe('folderSizeIndexer', () => { await fs.mkdir(target, { recursive: true }); for (let i = 0; i < 45; i += 1) { - // eslint-disable-next-line no-await-in-loop await fs.writeFile(path.join(target, `file-${i}`), Buffer.alloc(1)); } @@ -405,14 +404,14 @@ describe('folderSizeIndexer', () => { }); ctx = { env }; - const { getDb } = env.requireFresh('src/services/db'); + const { getIndexDb } = env.requireFresh('src/services/indexDb'); const folderSizeIndex = env.requireFresh('src/services/folderSizeIndex'); const scope = { root: env.volumeDir, label: 'volume' }; const legacy = path.join(scope.root, 'Legacy'); await fs.mkdir(path.join(legacy, 'nested'), { recursive: true }); await fs.writeFile(path.join(legacy, 'nested', 'payload.bin'), Buffer.alloc(42)); - const db = await getDb(); + const db = await getIndexDb(); const scannedAt = new Date().toISOString(); folderSizeIndex.upsertScanEntry(db, scope, { absolutePath: scope.root, @@ -535,7 +534,6 @@ describe('folderSizeIndexer', () => { let result; let slices = 0; do { - // eslint-disable-next-line no-await-in-loop result = await indexer.reconcile(db, scope, { mode: 'full', batch: 1, @@ -567,9 +565,7 @@ describe('folderSizeIndexer', () => { let dir = vol; for (let i = 0; i < depth; i += 1) { dir = path.join(dir, `L${i}`); - // eslint-disable-next-line no-await-in-loop await fs.mkdir(dir, { recursive: true }); - // eslint-disable-next-line no-await-in-loop await fs.writeFile(path.join(dir, 'f'), Buffer.alloc(10)); } @@ -630,9 +626,7 @@ describe('folderSizeIndexer', () => { // Build enough folders that the baseline yields multiple times. for (let i = 0; i < 60; i += 1) { - // eslint-disable-next-line no-await-in-loop await fs.mkdir(path.join(vol, `dir-${i}`, 'sub'), { recursive: true }); - // eslint-disable-next-line no-await-in-loop await fs.writeFile(path.join(vol, `dir-${i}`, 'sub', 'f'), Buffer.alloc(8)); } @@ -640,7 +634,6 @@ describe('folderSizeIndexer', () => { const heartbeat = []; const beat = async () => { for (let i = 0; i < 20; i += 1) { - // eslint-disable-next-line no-await-in-loop await new Promise((resolve) => setTimeout(resolve, 2)); heartbeat.push(baselineDone); } diff --git a/frontend/.eslintrc.cjs b/frontend/.eslintrc.cjs index a22640dcd..76300276a 100644 --- a/frontend/.eslintrc.cjs +++ b/frontend/.eslintrc.cjs @@ -5,4 +5,11 @@ module.exports = { parserOptions: { ecmaVersion: 'latest', }, + // `ecmaVersion: latest` sets the syntax and not what exists at run time, so + // `globalThis` — standard since ES2020, and what a store reaches for when it has to + // touch a timer or a listener the browser owns — read as an undefined name. + env: { + browser: true, + es2022: true, + }, }; diff --git a/frontend/src/stores/folderSize.js b/frontend/src/stores/folderSize.js index 90de52318..fb9f6d187 100644 --- a/frontend/src/stores/folderSize.js +++ b/frontend/src/stores/folderSize.js @@ -53,18 +53,18 @@ export const useFolderSizeStore = defineStore('folderSize', () => { queuedRefresh = false; if (refreshTimer) { - window.clearTimeout(refreshTimer); + globalThis.clearTimeout(refreshTimer); refreshTimer = null; } if (dirtyRefreshTimer) { - window.clearTimeout(dirtyRefreshTimer); + globalThis.clearTimeout(dirtyRefreshTimer); dirtyRefreshTimer = null; } dirtyRefreshDelay = DIRTY_REFRESH_INITIAL_MS; for (const timer of pendingManualRefreshes.values()) { - window.clearTimeout(timer); + globalThis.clearTimeout(timer); } pendingManualRefreshes.clear(); }; @@ -120,14 +120,14 @@ export const useFolderSizeStore = defineStore('folderSize', () => { if (!dirtyRefreshTimer) { const delay = dirtyRefreshDelay; dirtyRefreshDelay = Math.min(dirtyRefreshDelay * 2, DIRTY_REFRESH_MAX_MS); - dirtyRefreshTimer = window.setTimeout(() => { + dirtyRefreshTimer = globalThis.setTimeout(() => { dirtyRefreshTimer = null; refresh({ force: true }).catch(() => {}); }, delay); } } else { if (dirtyRefreshTimer) { - window.clearTimeout(dirtyRefreshTimer); + globalThis.clearTimeout(dirtyRefreshTimer); dirtyRefreshTimer = null; } dirtyRefreshDelay = DIRTY_REFRESH_INITIAL_MS; @@ -208,9 +208,9 @@ export const useFolderSizeStore = defineStore('folderSize', () => { } if (refreshTimer) { - window.clearTimeout(refreshTimer); + globalThis.clearTimeout(refreshTimer); } - refreshTimer = window.setTimeout(() => { + refreshTimer = globalThis.setTimeout(() => { refreshTimer = null; refresh({ force }).catch(() => {}); }, delayMs); @@ -245,11 +245,11 @@ export const useFolderSizeStore = defineStore('folderSize', () => { pendingManualRefreshes.delete(path); return; } - const timer = window.setTimeout(poll, MANUAL_REFRESH_POLL_MS); + const timer = globalThis.setTimeout(poll, MANUAL_REFRESH_POLL_MS); pendingManualRefreshes.set(path, timer); }; - const timer = window.setTimeout(poll, MANUAL_REFRESH_POLL_MS); + const timer = globalThis.setTimeout(poll, MANUAL_REFRESH_POLL_MS); pendingManualRefreshes.set(path, timer); };