mirror of
https://github.com/yu-i-i/overleaf-cep.git
synced 2026-11-12 13:13:45 +01:00
[monorepo] read empty content from its hash, in a shared blob store base (#36831)
The hash of empty content is a property of the content rather than of anything stored: every project's empty file has it, whether or not a blob was ever written under it, and a file may reference it before anything has. Eight blob stores were each paying a lookup -- a database query, an HTTP request, a read of a cache that offline mode may be the only thing standing in for -- to be told what the hash already says, and clsi had already reinvented the answer on its own. overleaf-editor-core gains BlobStoreBase, which answers getString for that hash and delegates the rest to the fetchString its subclasses provide. getObject comes with it: four stores spelled out the same JSON.parse of getString, and a fifth wrapped it in a log line. Adopted by history-v1's BlobStore, BackupBlobStore, HashCheckBlobStore and recover_zip's RecoveryBlobStore, by project-history's BlobStore, by clsi's BlobStore, and by web's HistoryBlobStore, SimpleBlobStore and FrontendBlobStore. history-v1 keeps its own answers in getStream, getBlob and getBlobs, which no other store implements. Writing and copying a blob are untouched, so a project that wrote empty content still has a blob for it and a read answers the same either way. Two paths could not handle a project holding a tracked change or a comment at all. Loading such a file needs the ranges blob its rangesHash points at, and hashing one again needs to write it back, so a store missing getObject or putObject fails outright: - recover_zip died on `blobStore.getObject is not a function`, making every such project unrecoverable. The test covering it asserts a tracked delete is absent from the zip, which the script already asked for and never reached. - GET /projects/:project_id/latest/hashed_content returned 500 from the same cause, then would have hit a missing putObject in snapshot.store. HashCheckBlobStore becomes a class over the base and gains putObject. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> GitOrigin-RevId: c84f62465beff6cf77ffac26aa772984e1b554cb
This commit is contained in:
committed by
Copybot
co-authored by
Claude Opus 5
parent
7608d738a6
commit
6caa5fc3a3
@@ -2,6 +2,7 @@ const AddCommentOperation = require('./lib/operation/add_comment_operation')
|
||||
const Author = require('./lib/author')
|
||||
const AuthorList = require('./lib/author_list')
|
||||
const Blob = require('./lib/blob')
|
||||
const BlobStoreBase = require('./lib/blob_store_base')
|
||||
const Change = require('./lib/change')
|
||||
const ChangeRequest = require('./lib/change_request')
|
||||
const ChangeNote = require('./lib/change_note')
|
||||
@@ -66,6 +67,7 @@ exports.AddCommentOperation = AddCommentOperation
|
||||
exports.Author = Author
|
||||
exports.AuthorList = AuthorList
|
||||
exports.Blob = Blob
|
||||
exports.BlobStoreBase = BlobStoreBase
|
||||
exports.Change = Change
|
||||
exports.ChangeRequest = ChangeRequest
|
||||
exports.ChangeNote = ChangeNote
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
// @ts-check
|
||||
'use strict'
|
||||
|
||||
const Blob = require('./blob')
|
||||
|
||||
/**
|
||||
* The parts of reading a blob that every blob store answers the same way.
|
||||
*
|
||||
* The hash of empty content is a property of the content rather than of anything
|
||||
* stored: every project's empty file has it, whether or not anything ever wrote a
|
||||
* blob under it, and a file may reference it before anything has. So no store has
|
||||
* to look it up, and every store that did was paying a lookup -- a database query,
|
||||
* an HTTP request, a read of a cache that offline mode may be the only thing
|
||||
* standing in for -- to be told what the hash already says. Deserializing a blob
|
||||
* that holds JSON is likewise the same wherever it is read from.
|
||||
*
|
||||
* Implementations extend this and provide `fetchString`, which is only ever called
|
||||
* for a hash that needs fetching. Spelling either of these out in each store
|
||||
* instead is a line per store, and the store written next is the one that forgets.
|
||||
*
|
||||
* `getObject` inherits the empty-hash answer through `getString`, so it parses `''`
|
||||
* and fails. That is right: a ranges blob is written from `JSON.stringify`, which
|
||||
* never yields empty content, so a hash reaching `getObject` is one something
|
||||
* stored. A store that reads or repairs its JSON differently overrides it.
|
||||
*/
|
||||
class BlobStoreBase {
|
||||
/**
|
||||
* Fetch a blob's content as a string.
|
||||
*
|
||||
* @param {string} hash hexadecimal SHA-1 hash
|
||||
* @return {Promise<string>}
|
||||
*/
|
||||
async getString(hash) {
|
||||
if (hash === Blob.EMPTY_HASH) {
|
||||
return ''
|
||||
}
|
||||
return await this.fetchString(hash)
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a blob holding JSON and deserialize it.
|
||||
*
|
||||
* @template [T=unknown]
|
||||
* @param {string} hash hexadecimal SHA-1 hash
|
||||
* @return {Promise<T>}
|
||||
*/
|
||||
async getObject(hash) {
|
||||
return /** @type {T} */ (JSON.parse(await this.getString(hash)))
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a blob's content as a string from wherever this store keeps it.
|
||||
*
|
||||
* Called only for a hash that something needs to have stored, so an
|
||||
* implementation may treat a miss as an error.
|
||||
*
|
||||
* @abstract
|
||||
* @param {string} hash hexadecimal SHA-1 hash
|
||||
* @return {Promise<string>}
|
||||
*/
|
||||
async fetchString(hash) {
|
||||
throw new Error(
|
||||
`${this.constructor.name} does not implement fetchString(hash)`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = BlobStoreBase
|
||||
@@ -0,0 +1,95 @@
|
||||
'use strict'
|
||||
|
||||
const { expect } = require('chai')
|
||||
|
||||
const { Blob, BlobStoreBase } = require('../..')
|
||||
|
||||
class TestBlobStore extends BlobStoreBase {
|
||||
constructor(contentByHash = {}) {
|
||||
super()
|
||||
this.contentByHash = contentByHash
|
||||
this.fetched = []
|
||||
}
|
||||
|
||||
async fetchString(hash) {
|
||||
this.fetched.push(hash)
|
||||
if (!(hash in this.contentByHash)) {
|
||||
throw new Error(`nothing stored under ${hash}`)
|
||||
}
|
||||
return this.contentByHash[hash]
|
||||
}
|
||||
}
|
||||
|
||||
const HASH = 'a1b2c3d4e5f60718293a4b5c6d7e8f9012345678'
|
||||
|
||||
describe('BlobStoreBase', function () {
|
||||
describe('getString', function () {
|
||||
it('fetches the content of a hash something stored', async function () {
|
||||
const blobStore = new TestBlobStore({ [HASH]: 'hello world' })
|
||||
|
||||
expect(await blobStore.getString(HASH)).to.equal('hello world')
|
||||
expect(blobStore.fetched).to.deep.equal([HASH])
|
||||
})
|
||||
|
||||
it('answers the hash of empty content without fetching', async function () {
|
||||
// The store below has nothing stored under it and throws when asked, which is
|
||||
// the point: the hash says what the content is, so no store has to be asked.
|
||||
const blobStore = new TestBlobStore()
|
||||
|
||||
expect(await blobStore.getString(Blob.EMPTY_HASH)).to.equal('')
|
||||
expect(blobStore.fetched).to.have.length(0)
|
||||
})
|
||||
|
||||
it('passes on what the implementation throws', async function () {
|
||||
const blobStore = new TestBlobStore()
|
||||
|
||||
try {
|
||||
await blobStore.getString(HASH)
|
||||
expect.fail('should have thrown')
|
||||
} catch (err) {
|
||||
expect(err.message).to.equal(`nothing stored under ${HASH}`)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('getObject', function () {
|
||||
it('deserializes the content of the hash', async function () {
|
||||
const blobStore = new TestBlobStore({
|
||||
[HASH]: '{"comments":[],"trackedChanges":[]}',
|
||||
})
|
||||
|
||||
expect(await blobStore.getObject(HASH)).to.deep.equal({
|
||||
comments: [],
|
||||
trackedChanges: [],
|
||||
})
|
||||
})
|
||||
|
||||
it('fails on the hash of empty content, which is not JSON', async function () {
|
||||
// Nothing writes an empty ranges blob -- JSON.stringify never yields empty
|
||||
// content -- so a hash reaching here is one something stored.
|
||||
const blobStore = new TestBlobStore()
|
||||
|
||||
try {
|
||||
await blobStore.getObject(Blob.EMPTY_HASH)
|
||||
expect.fail('should have thrown')
|
||||
} catch (err) {
|
||||
expect(err).to.be.an.instanceof(SyntaxError)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('fetchString', function () {
|
||||
it('names the store that has not implemented it', async function () {
|
||||
class IncompleteBlobStore extends BlobStoreBase {}
|
||||
|
||||
try {
|
||||
await new IncompleteBlobStore().getString(HASH)
|
||||
expect.fail('should have thrown')
|
||||
} catch (err) {
|
||||
expect(err.message).to.equal(
|
||||
'IncompleteBlobStore does not implement fetchString(hash)'
|
||||
)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user