From d489e35782420880708a4918c216e767c897b6fe Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 8 May 2025 17:08:38 +0200 Subject: [PATCH] [web] emit event when synctex mapping was downloaded from clsi-cache (#25424) * [clsi] tell frontend when synctex mapping was downloaded from clsi-cache * [web] emit event when synctex mapping was downloaded from clsi-cache GitOrigin-RevId: 1f6b7e0faaa7dd76449aad566802da971a4cf9ed --- services/clsi/app/js/CompileController.js | 6 +- services/clsi/app/js/CompileManager.js | 60 ++++++++--- .../acceptance/js/AllowedImageNamesTests.js | 2 + .../clsi/test/acceptance/js/SynctexTests.js | 2 + .../test/unit/js/CompileControllerTests.js | 6 +- .../clsi/test/unit/js/CompileManagerTests.js | 102 ++++++++++++++++-- .../features/pdf-preview/hooks/use-synctex.ts | 13 +++ 7 files changed, 160 insertions(+), 31 deletions(-) diff --git a/services/clsi/app/js/CompileController.js b/services/clsi/app/js/CompileController.js index 138801890a..e102b19dad 100644 --- a/services/clsi/app/js/CompileController.js +++ b/services/clsi/app/js/CompileController.js @@ -204,12 +204,13 @@ function syncFromCode(req, res, next) { line, column, { imageName, editorId, buildId, compileFromClsiCache }, - function (error, pdfPositions) { + function (error, pdfPositions, downloadedFromCache) { if (error) { return next(error) } res.json({ pdf: pdfPositions, + downloadedFromCache, }) } ) @@ -229,12 +230,13 @@ function syncFromPdf(req, res, next) { h, v, { imageName, editorId, buildId, compileFromClsiCache }, - function (error, codePositions) { + function (error, codePositions, downloadedFromCache) { if (error) { return next(error) } res.json({ code: codePositions, + downloadedFromCache, }) } ) diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index b65fb3cd02..1b66927412 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -23,6 +23,7 @@ const { downloadLatestCompileCache, downloadOutputDotSynctexFromCompileCache, } = require('./CLSICacheHandler') +const { callbackifyMultiResult } = require('@overleaf/promise-utils') const COMPILE_TIME_BUCKETS = [ // NOTE: These buckets are locked in per metric name. @@ -447,12 +448,20 @@ async function syncFromCode(projectId, userId, filename, line, column, opts) { '-o', outputFilePath, ] - const stdout = await _runSynctex(projectId, userId, command, opts) + const { stdout, downloadedFromCache } = await _runSynctex( + projectId, + userId, + command, + opts + ) logger.debug( { projectId, userId, filename, line, column, command, stdout }, 'synctex code output' ) - return SynctexOutputParser.parseViewOutput(stdout) + return { + codePositions: SynctexOutputParser.parseViewOutput(stdout), + downloadedFromCache, + } } async function syncFromPdf(projectId, userId, page, h, v, opts) { @@ -465,9 +474,17 @@ async function syncFromPdf(projectId, userId, page, h, v, opts) { '-o', `${page}:${h}:${v}:${outputFilePath}`, ] - const stdout = await _runSynctex(projectId, userId, command, opts) + const { stdout, downloadedFromCache } = await _runSynctex( + projectId, + userId, + command, + opts + ) logger.debug({ projectId, userId, page, h, v, stdout }, 'synctex pdf output') - return SynctexOutputParser.parseEditOutput(stdout, baseDir) + return { + pdfPositions: SynctexOutputParser.parseEditOutput(stdout, baseDir), + downloadedFromCache, + } } async function _checkFileExists(dir, filename) { @@ -522,9 +539,10 @@ async function _runSynctex(projectId, userId, command, opts) { return await OutputCacheManager.promises.queueDirOperation( outputDir, /** - * @return {Promise} + * @return {Promise<{stdout: string, downloadedFromCache: boolean}>} */ async () => { + let downloadedFromCache = false try { await _checkFileExists(directory, 'output.synctex.gz') } catch (err) { @@ -535,13 +553,14 @@ async function _runSynctex(projectId, userId, command, opts) { buildId ) { try { - await downloadOutputDotSynctexFromCompileCache( - projectId, - userId, - editorId, - buildId, - directory - ) + downloadedFromCache = + await downloadOutputDotSynctexFromCompileCache( + projectId, + userId, + editorId, + buildId, + directory + ) } catch (err) { logger.warn( { err, projectId, userId, editorId, buildId }, @@ -554,7 +573,7 @@ async function _runSynctex(projectId, userId, command, opts) { } } try { - const output = await CommandRunner.promises.run( + const { stdout } = await CommandRunner.promises.run( compileName, command, directory, @@ -563,7 +582,10 @@ async function _runSynctex(projectId, userId, command, opts) { {}, compileGroup ) - return output.stdout + return { + stdout, + downloadedFromCache, + } } catch (error) { throw OError.tag(error, 'error running synctex', { command, @@ -686,8 +708,14 @@ module.exports = { stopCompile: callbackify(stopCompile), clearProject: callbackify(clearProject), clearExpiredProjects: callbackify(clearExpiredProjects), - syncFromCode: callbackify(syncFromCode), - syncFromPdf: callbackify(syncFromPdf), + syncFromCode: callbackifyMultiResult(syncFromCode, [ + 'codePositions', + 'downloadedFromCache', + ]), + syncFromPdf: callbackifyMultiResult(syncFromPdf, [ + 'pdfPositions', + 'downloadedFromCache', + ]), wordcount: callbackify(wordcount), promises: { doCompileWithLock, diff --git a/services/clsi/test/acceptance/js/AllowedImageNamesTests.js b/services/clsi/test/acceptance/js/AllowedImageNamesTests.js index 897f5d9c85..9cd7a65930 100644 --- a/services/clsi/test/acceptance/js/AllowedImageNamesTests.js +++ b/services/clsi/test/acceptance/js/AllowedImageNamesTests.js @@ -109,6 +109,7 @@ Hello world width: 343.71106, }, ], + downloadedFromCache: false, }) done() } @@ -146,6 +147,7 @@ Hello world expect(error).to.not.exist expect(result).to.deep.equal({ code: [{ file: 'main.tex', line: 3, column: -1 }], + downloadedFromCache: false, }) done() } diff --git a/services/clsi/test/acceptance/js/SynctexTests.js b/services/clsi/test/acceptance/js/SynctexTests.js index 5ba5bb5b5f..049f260259 100644 --- a/services/clsi/test/acceptance/js/SynctexTests.js +++ b/services/clsi/test/acceptance/js/SynctexTests.js @@ -67,6 +67,7 @@ Hello world width: 343.71106, }, ], + downloadedFromCache: false, }) return done() } @@ -87,6 +88,7 @@ Hello world } expect(codePositions).to.deep.equal({ code: [{ file: 'main.tex', line: 3, column: -1 }], + downloadedFromCache: false, }) return done() } diff --git a/services/clsi/test/unit/js/CompileControllerTests.js b/services/clsi/test/unit/js/CompileControllerTests.js index 506b5f02dd..2ac8d9c2d7 100644 --- a/services/clsi/test/unit/js/CompileControllerTests.js +++ b/services/clsi/test/unit/js/CompileControllerTests.js @@ -410,7 +410,7 @@ describe('CompileController', function () { this.CompileManager.syncFromCode = sinon .stub() - .yields(null, (this.pdfPositions = ['mock-positions'])) + .yields(null, (this.pdfPositions = ['mock-positions']), true) this.CompileController.syncFromCode(this.req, this.res, this.next) }) @@ -430,6 +430,7 @@ describe('CompileController', function () { this.res.json .calledWith({ pdf: this.pdfPositions, + downloadedFromCache: true, }) .should.equal(true) }) @@ -451,7 +452,7 @@ describe('CompileController', function () { this.CompileManager.syncFromPdf = sinon .stub() - .yields(null, (this.codePositions = ['mock-positions'])) + .yields(null, (this.codePositions = ['mock-positions']), true) this.CompileController.syncFromPdf(this.req, this.res, this.next) }) @@ -465,6 +466,7 @@ describe('CompileController', function () { this.res.json .calledWith({ code: this.codePositions, + downloadedFromCache: true, }) .should.equal(true) }) diff --git a/services/clsi/test/unit/js/CompileManagerTests.js b/services/clsi/test/unit/js/CompileManagerTests.js index 33a43ae029..30ef538ac3 100644 --- a/services/clsi/test/unit/js/CompileManagerTests.js +++ b/services/clsi/test/unit/js/CompileManagerTests.js @@ -35,7 +35,7 @@ describe('CompileManager', function () { build: 1234, }, ] - this.buildId = 'build-id-123' + this.buildId = '00000000000-0000000000000000' this.commandOutput = 'Dummy output' this.compileBaseDir = '/compile/dir' this.outputBaseDir = '/output/dir' @@ -61,6 +61,8 @@ describe('CompileManager', function () { }, } this.OutputCacheManager = { + BUILD_REGEX: /^[0-9a-f]+-[0-9a-f]+$/, + CACHE_SUBDIR: 'generated-files', promises: { queueDirOperation: sinon.stub().callsArg(1), saveOutputFiles: sinon @@ -88,9 +90,10 @@ describe('CompileManager', function () { execFile: sinon.stub().yields(), } this.CommandRunner = { + canRunSyncTeXInOutputDir: sinon.stub().returns(false), promises: { run: sinon.stub().callsFake((_1, _2, _3, _4, _5, _6, compileGroup) => { - if (compileGroup === 'synctex') { + if (compileGroup === 'synctex' || compileGroup === 'synctex-output') { return Promise.resolve({ stdout: this.commandOutput }) } else { return Promise.resolve({ @@ -141,6 +144,12 @@ describe('CompileManager', function () { .withArgs(Path.join(this.compileDir, 'output.synctex.gz')) .resolves(this.fileStats) + this.CLSICacheHandler = { + notifyCLSICacheAboutBuild: sinon.stub(), + downloadLatestCompileCache: sinon.stub().resolves(), + downloadOutputDotSynctexFromCompileCache: sinon.stub().resolves(), + } + this.CompileManager = SandboxedModule.require(MODULE_PATH, { requires: { './LatexRunner': this.LatexRunner, @@ -161,11 +170,7 @@ describe('CompileManager', function () { './LockManager': this.LockManager, './SynctexOutputParser': this.SynctexOutputParser, 'fs/promises': this.fsPromises, - './CLSICacheHandler': { - notifyCLSICacheAboutBuild: sinon.stub(), - downloadLatestCompileCache: sinon.stub().resolves(), - downloadOutputDotSynctexFromCompileCache: sinon.stub().resolves(), - }, + './CLSICacheHandler': this.CLSICacheHandler, }, }) }) @@ -462,12 +467,83 @@ describe('CompileManager', function () { this.compileDir, this.Settings.clsi.docker.image, 60000, - {} + {}, + 'synctex' ) }) it('should return the parsed output', function () { - expect(this.result).to.deep.equal(this.records) + expect(this.result).to.deep.equal({ + codePositions: this.records, + downloadedFromCache: false, + }) + }) + }) + + describe('from cache in docker', function () { + beforeEach(async function () { + this.CommandRunner.canRunSyncTeXInOutputDir.returns(true) + this.Settings.path.synctexBaseDir + .withArgs(`${this.projectId}-${this.userId}`) + .returns('/compile') + + const errNotFound = new Error() + errNotFound.code = 'ENOENT' + this.outputDir = `${this.outputBaseDir}/${this.projectId}-${this.userId}/${this.OutputCacheManager.CACHE_SUBDIR}/${this.buildId}` + const filename = Path.join(this.outputDir, 'output.synctex.gz') + this.fsPromises.stat + .withArgs(this.outputDir) + .onFirstCall() + .rejects(errNotFound) + this.fsPromises.stat + .withArgs(this.outputDir) + .onSecondCall() + .resolves(this.dirStats) + this.fsPromises.stat.withArgs(filename).resolves(this.fileStats) + this.CLSICacheHandler.downloadOutputDotSynctexFromCompileCache.resolves( + true + ) + this.result = await this.CompileManager.promises.syncFromCode( + this.projectId, + this.userId, + this.filename, + this.line, + this.column, + { + imageName: 'image', + editorId: '00000000-0000-0000-0000-000000000000', + buildId: this.buildId, + compileFromClsiCache: true, + } + ) + }) + + it('should run in output dir', function () { + const outputFilePath = '/compile/output.pdf' + const inputFilePath = `/compile/${this.filename}` + expect(this.CommandRunner.promises.run).to.have.been.calledWith( + `${this.projectId}-${this.userId}`, + [ + 'synctex', + 'view', + '-i', + `${this.line}:${this.column}:${inputFilePath}`, + '-o', + outputFilePath, + ], + this.outputDir, + 'image', + 60000, + {}, + 'synctex-output' + ) + }) + + it('should return the parsed output', function () { + expect(this.result).to.deep.equal({ + codePositions: this.records, + downloadedFromCache: true, + }) }) }) @@ -500,7 +576,8 @@ describe('CompileManager', function () { this.compileDir, customImageName, 60000, - {} + {}, + 'synctex' ) }) }) @@ -544,7 +621,10 @@ describe('CompileManager', function () { }) it('should return the parsed output', function () { - expect(this.result).to.deep.equal(this.records) + expect(this.result).to.deep.equal({ + pdfPositions: this.records, + downloadedFromCache: false, + }) }) }) diff --git a/services/web/frontend/js/features/pdf-preview/hooks/use-synctex.ts b/services/web/frontend/js/features/pdf-preview/hooks/use-synctex.ts index 686d988dee..77986bfeac 100644 --- a/services/web/frontend/js/features/pdf-preview/hooks/use-synctex.ts +++ b/services/web/frontend/js/features/pdf-preview/hooks/use-synctex.ts @@ -18,6 +18,7 @@ import { CursorPosition } from '@/features/ide-react/types/cursor-position' import { isValidTeXFile } from '@/main/is-valid-tex-file' import { PdfScrollPosition } from '@/shared/hooks/use-pdf-scroll-position' import { showFileErrorToast } from '@/features/pdf-preview/components/synctex-toasts' +import { sendMB } from '@/infrastructure/event-tracking' export default function useSynctex(): { syncToPdf: () => void @@ -115,6 +116,12 @@ export default function useSynctex(): { .then(data => { setShowLogs(false) setHighlights(data.pdf) + if (data.downloadedFromCache) { + sendMB('synctex-downloaded-from-cache', { + projectId, + method: 'code', + }) + } }) .catch(debugConsole.error) .finally(() => { @@ -223,6 +230,12 @@ export default function useSynctex(): { .then(data => { const [{ file, line }] = data.code goToCodeLine(file, line) + if (data.downloadedFromCache) { + sendMB('synctex-downloaded-from-cache', { + projectId, + method: 'pdf', + }) + } }) .catch(debugConsole.error) .finally(() => {