From 85f4966d49742ae5dfc0845f4c7eb53908865a85 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 30 Nov 2024 16:36:22 +0100 Subject: [PATCH] Merge pull request #22256 from overleaf/revert-22239-jpa-file-url-helper Revert "[web] add helper for getting file URL from history w/ filestore fallback" GitOrigin-RevId: 3feecb5291f574de058b51efd5c799e07d5ec851 --- .../src/Features/History/HistoryURLHelper.js | 19 ------- .../Features/References/ReferencesHandler.mjs | 20 ++++--- .../ThirdPartyDataStore/TpdsUpdateSender.js | 18 +++---- .../TpdsUpdateSenderTests.js | 53 ------------------- 4 files changed, 21 insertions(+), 89 deletions(-) delete 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 deleted file mode 100644 index 398f1496ae..0000000000 --- a/services/web/app/src/Features/History/HistoryURLHelper.js +++ /dev/null @@ -1,19 +0,0 @@ -// 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 5bce9759e5..b3c39af45f 100644 --- a/services/web/app/src/Features/References/ReferencesHandler.mjs +++ b/services/web/app/src/Features/References/ReferencesHandler.mjs @@ -24,7 +24,6 @@ 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 @@ -39,6 +38,18 @@ 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) { @@ -168,12 +179,7 @@ export default ReferencesHandler = { ReferencesHandler._buildDocUrl(projectId, docId) ) const bibFileUrls = fileRefs.map(fileRef => - HistoryURLHelper.projectHistoryURLWithFilestoreFallback( - settings, - projectId, - historyId, - fileRef - ) + ReferencesHandler._buildFileUrl(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 0e624dbc43..dda525ddd4 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js @@ -6,7 +6,6 @@ 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 @@ -82,13 +81,12 @@ async function addFile(params) { folderId, } = params // Go through project-history to avoid the need for handling history-v1 authentication. - const { url, fallbackURL } = - HistoryURLHelper.projectHistoryURLWithFilestoreFallback( - settings, - projectId, - historyId, - { _id: fileId, hash } - ) + 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}`) await addEntity({ projectId, @@ -96,8 +94,8 @@ async function addFile(params) { projectName, rev, folderId, - streamOrigin: url, - streamFallback: fallbackURL, + streamOrigin, + streamFallback, 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 86818f6eeb..81d7c1b42b 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js +++ b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js @@ -116,59 +116,6 @@ 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