From b9a56afe4c8a2d5f281f8b6a3f9e38962897124f Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 29 Nov 2024 13:48:43 +0100 Subject: [PATCH] Merge pull request #22239 from overleaf/jpa-file-url-helper [web] add helper for getting file URL from history w/ filestore fallback GitOrigin-RevId: d77e6f1565fab407e1004a02c4bfdfe03a0214d7 --- .../src/Features/History/HistoryURLHelper.js | 19 +++++++ .../Features/References/ReferencesHandler.mjs | 20 +++---- .../ThirdPartyDataStore/TpdsUpdateSender.js | 18 ++++--- .../TpdsUpdateSenderTests.js | 53 +++++++++++++++++++ 4 files changed, 89 insertions(+), 21 deletions(-) create mode 100644 services/web/app/src/Features/History/HistoryURLHelper.js diff --git a/services/web/app/src/Features/History/HistoryURLHelper.js b/services/web/app/src/Features/History/HistoryURLHelper.js new file mode 100644 index 0000000000..398f1496ae --- /dev/null +++ b/services/web/app/src/Features/History/HistoryURLHelper.js @@ -0,0 +1,19 @@ +// Pass settings to enable consistent unit tests from .js and .mjs modules +function projectHistoryURLWithFilestoreFallback( + Settings, + projectId, + historyId, + fileRef +) { + const filestoreURL = `${Settings.apis.filestore.url}/project/${projectId}/file/${fileRef._id}` + if (fileRef.hash) { + return { + url: `${Settings.apis.project_history.url}/project/${historyId}/blob/${fileRef.hash}`, + fallbackURL: filestoreURL, + } + } else { + return { url: filestoreURL } + } +} + +module.exports = { projectHistoryURLWithFilestoreFallback } diff --git a/services/web/app/src/Features/References/ReferencesHandler.mjs b/services/web/app/src/Features/References/ReferencesHandler.mjs index b3c39af45f..5bce9759e5 100644 --- a/services/web/app/src/Features/References/ReferencesHandler.mjs +++ b/services/web/app/src/Features/References/ReferencesHandler.mjs @@ -24,6 +24,7 @@ import _ from 'lodash' import Async from 'async' import Errors from '../Errors/Errors.js' import { promisify } from '@overleaf/promise-utils' +import HistoryURLHelper from '../History/HistoryURLHelper.js' let ReferencesHandler @@ -38,18 +39,6 @@ export default ReferencesHandler = { } }, - _buildFileUrl(projectId, historyId, fileRef) { - const filestoreURL = `${settings.apis.filestore.url}/project/${projectId}/file/${fileRef._id}` - if (fileRef.hash) { - return { - url: `${settings.apis.project_history.url}/project/${historyId}/blob/${fileRef.hash}`, - fallbackURL: filestoreURL, - } - } else { - return { url: filestoreURL } - } - }, - _findBibFileRefs(project) { const fileRefs = [] function _process(folder) { @@ -179,7 +168,12 @@ export default ReferencesHandler = { ReferencesHandler._buildDocUrl(projectId, docId) ) const bibFileUrls = fileRefs.map(fileRef => - ReferencesHandler._buildFileUrl(projectId, historyId, fileRef) + HistoryURLHelper.projectHistoryURLWithFilestoreFallback( + settings, + projectId, + historyId, + fileRef + ) ) const sourceURLs = bibDocUrls.concat(bibFileUrls) return request.post( diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js index dda525ddd4..0e624dbc43 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js @@ -6,6 +6,7 @@ const metrics = require('@overleaf/metrics') const Path = require('path') const { fetchNothing } = require('@overleaf/fetch-utils') const settings = require('@overleaf/settings') +const HistoryURLHelper = require('../History/HistoryURLHelper') const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter').promises @@ -81,12 +82,13 @@ async function addFile(params) { folderId, } = params // Go through project-history to avoid the need for handling history-v1 authentication. - const streamOrigin = - settings.apis.project_history.url + - Path.join(`/project/${historyId}/blob/${hash}`) - const streamFallback = - settings.apis.filestore.url + - Path.join(`/project/${projectId}`, `/file/${fileId}`) + const { url, fallbackURL } = + HistoryURLHelper.projectHistoryURLWithFilestoreFallback( + settings, + projectId, + historyId, + { _id: fileId, hash } + ) await addEntity({ projectId, @@ -94,8 +96,8 @@ async function addFile(params) { projectName, rev, folderId, - streamOrigin, - streamFallback, + streamOrigin: url, + streamFallback: fallbackURL, entityId: fileId, entityType: 'file', }) diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js index 81d7c1b42b..86818f6eeb 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js +++ b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js @@ -116,6 +116,59 @@ describe('TpdsUpdateSender', function () { }) it('queues a post the file with user and file id', async function () { + const fileId = '4545345' + const hash = undefined + const historyId = 91525 + const path = '/some/path/here.jpg' + + await this.TpdsUpdateSender.promises.addFile({ + projectId, + historyId, + fileId, + hash, + path, + projectName, + }) + + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: userId, + method: 'pipeStreamFrom', + job: { + method: 'post', + streamOrigin: `${filestoreUrl}/project/${projectId}/file/${fileId}`, + uri: `${thirdPartyDataStoreApiUrl}/user/${userId}/entity/${encodeURIComponent( + projectName + )}${encodeURIComponent(path)}`, + headers: {}, + }, + }, + } + ) + + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: collaberatorRef, + }, + } + ) + + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: readOnlyRef, + job: {}, + }, + } + ) + }) + + it('queues a post the file with user and file id and hash', async function () { const fileId = '4545345' const hash = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' const historyId = 91525