mirror of
https://github.com/overleaf/overleaf.git
synced 2024-11-14 20:40:17 -05:00
Merge pull request #21375 from overleaf/jpa-if-none-match
[object-persistor] add support for ifNoneMatch=* in sendStream GitOrigin-RevId: 268f054ac1b6452105b02757cdec32bad00702fd
This commit is contained in:
Jakob Ackermann
Copybot
parent
a551a0e9f7
commit
7b3e39f63f
8 changed files with 122 additions and 9 deletions
|
|
@ -24,7 +24,7 @@
|
|||
"@overleaf/logger": "*",
|
||||
"@overleaf/metrics": "*",
|
||||
"@overleaf/o-error": "*",
|
||||
"aws-sdk": "^2.718.0",
|
||||
"aws-sdk": "^2.1691.0",
|
||||
"fast-crc32c": "overleaf/node-fast-crc32c#aae6b2a4c7a7a159395df9cc6c38dfde702d6f51",
|
||||
"glob": "^7.1.6",
|
||||
"range-parser": "^1.2.1",
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ class WriteError extends OError {}
|
|||
class ReadError extends OError {}
|
||||
class SettingsError extends OError {}
|
||||
class NotImplementedError extends OError {}
|
||||
class AlreadyWrittenError extends OError {}
|
||||
|
||||
module.exports = {
|
||||
NotFoundError,
|
||||
|
|
@ -12,4 +13,5 @@ module.exports = {
|
|||
ReadError,
|
||||
SettingsError,
|
||||
NotImplementedError,
|
||||
AlreadyWrittenError,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ const { pipeline } = require('stream/promises')
|
|||
const { promisify } = require('util')
|
||||
|
||||
const AbstractPersistor = require('./AbstractPersistor')
|
||||
const { ReadError, WriteError } = require('./Errors')
|
||||
const { ReadError, WriteError, NotImplementedError } = require('./Errors')
|
||||
const PersistorHelper = require('./PersistorHelper')
|
||||
|
||||
const glob = promisify(globCallbacks)
|
||||
|
|
@ -36,6 +36,14 @@ module.exports = class FSPersistor extends AbstractPersistor {
|
|||
}
|
||||
|
||||
async sendStream(location, target, sourceStream, opts = {}) {
|
||||
if (opts.ifNoneMatch === '*') {
|
||||
// The standard library only has fs.rename(), which does not support exclusive flags.
|
||||
// Refuse to act on this write operation.
|
||||
throw new NotImplementedError(
|
||||
'Overwrite protection required by caller, but it is not available is FS backend. Configure GCS or S3 backend instead, get in touch with support for further information.'
|
||||
)
|
||||
}
|
||||
|
||||
const targetPath = this._getFsPath(location, target)
|
||||
|
||||
try {
|
||||
|
|
@ -55,7 +63,7 @@ module.exports = class FSPersistor extends AbstractPersistor {
|
|||
throw PersistorHelper.wrapError(
|
||||
err,
|
||||
'failed to write stream',
|
||||
{ location, target },
|
||||
{ location, target, ifNoneMatch: opts.ifNoneMatch },
|
||||
WriteError
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -78,10 +78,14 @@ module.exports = class GcsPersistor extends AbstractPersistor {
|
|||
writeOptions.metadata = writeOptions.metadata || {}
|
||||
writeOptions.metadata.contentEncoding = opts.contentEncoding
|
||||
}
|
||||
const fileOptions = {}
|
||||
if (opts.ifNoneMatch === '*') {
|
||||
fileOptions.generation = 0
|
||||
}
|
||||
|
||||
const uploadStream = this.storage
|
||||
.bucket(bucketName)
|
||||
.file(key)
|
||||
.file(key, fileOptions)
|
||||
.createWriteStream(writeOptions)
|
||||
|
||||
await pipeline(readStream, observer, uploadStream)
|
||||
|
|
@ -97,7 +101,7 @@ module.exports = class GcsPersistor extends AbstractPersistor {
|
|||
throw PersistorHelper.wrapError(
|
||||
err,
|
||||
'upload to GCS failed',
|
||||
{ bucketName, key },
|
||||
{ bucketName, key, ifNoneMatch: opts.ifNoneMatch },
|
||||
WriteError
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ const Stream = require('stream')
|
|||
const { pipeline } = require('stream/promises')
|
||||
const Logger = require('@overleaf/logger')
|
||||
const Metrics = require('@overleaf/metrics')
|
||||
const { WriteError, NotFoundError } = require('./Errors')
|
||||
const { WriteError, NotFoundError, AlreadyWrittenError } = require('./Errors')
|
||||
|
||||
const _128KiB = 128 * 1024
|
||||
const TIMING_BUCKETS = [
|
||||
|
|
@ -146,6 +146,13 @@ function wrapError(error, message, params, ErrorType) {
|
|||
(error.response && error.response.statusCode === 404)
|
||||
) {
|
||||
return new NotFoundError('no such file', params, error)
|
||||
} else if (
|
||||
params.ifNoneMatch === '*' &&
|
||||
(error.code === 'PreconditionFailed' ||
|
||||
error.response?.statusCode === 412 ||
|
||||
error instanceof AlreadyWrittenError)
|
||||
) {
|
||||
return new AlreadyWrittenError(message, params, error)
|
||||
} else {
|
||||
return new ErrorType(message, params, error)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -52,6 +52,9 @@ module.exports = class S3Persistor extends AbstractPersistor {
|
|||
if (opts.contentEncoding) {
|
||||
uploadOptions.ContentEncoding = opts.contentEncoding
|
||||
}
|
||||
if (opts.ifNoneMatch === '*') {
|
||||
uploadOptions.IfNoneMatch = '*'
|
||||
}
|
||||
|
||||
// if we have an md5 hash, pass this to S3 to verify the upload - otherwise
|
||||
// we rely on the S3 client's checksum calculation to validate the upload
|
||||
|
|
@ -69,7 +72,7 @@ module.exports = class S3Persistor extends AbstractPersistor {
|
|||
throw PersistorHelper.wrapError(
|
||||
err,
|
||||
'upload to S3 failed',
|
||||
{ bucketName, key },
|
||||
{ bucketName, key, ifNoneMatch: opts.ifNoneMatch },
|
||||
WriteError
|
||||
)
|
||||
}
|
||||
|
|
|
|||
55
package-lock.json
generated
55
package-lock.json
generated
|
|
@ -258,7 +258,7 @@
|
|||
"@overleaf/logger": "*",
|
||||
"@overleaf/metrics": "*",
|
||||
"@overleaf/o-error": "*",
|
||||
"aws-sdk": "^2.718.0",
|
||||
"aws-sdk": "^2.1691.0",
|
||||
"fast-crc32c": "overleaf/node-fast-crc32c#aae6b2a4c7a7a159395df9cc6c38dfde702d6f51",
|
||||
"glob": "^7.1.6",
|
||||
"range-parser": "^1.2.1",
|
||||
|
|
@ -276,6 +276,27 @@
|
|||
"typescript": "^5.0.4"
|
||||
}
|
||||
},
|
||||
"libraries/object-persistor/node_modules/aws-sdk": {
|
||||
"version": "2.1691.0",
|
||||
"resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.1691.0.tgz",
|
||||
"integrity": "sha512-/F2YC+DlsY3UBM2Bdnh5RLHOPNibS/+IcjUuhP8XuctyrN+MlL+fWDAiela32LTDk7hMy4rx8MTgvbJ+0blO5g==",
|
||||
"hasInstallScript": true,
|
||||
"dependencies": {
|
||||
"buffer": "4.9.2",
|
||||
"events": "1.1.1",
|
||||
"ieee754": "1.1.13",
|
||||
"jmespath": "0.16.0",
|
||||
"querystring": "0.2.0",
|
||||
"sax": "1.2.1",
|
||||
"url": "0.10.3",
|
||||
"util": "^0.12.4",
|
||||
"uuid": "8.0.0",
|
||||
"xml2js": "0.6.2"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">= 10.0.0"
|
||||
}
|
||||
},
|
||||
"libraries/object-persistor/node_modules/gcp-metadata": {
|
||||
"version": "5.3.0",
|
||||
"resolved": "https://registry.npmjs.org/gcp-metadata/-/gcp-metadata-5.3.0.tgz",
|
||||
|
|
@ -337,6 +358,14 @@
|
|||
}
|
||||
}
|
||||
},
|
||||
"libraries/object-persistor/node_modules/uuid": {
|
||||
"version": "8.0.0",
|
||||
"resolved": "https://registry.npmjs.org/uuid/-/uuid-8.0.0.tgz",
|
||||
"integrity": "sha512-jOXGuXZAWdsTH7eZLtyXMqUb9EcWMGZNbL9YcGBJl4MH4nrxHmZJhEHvyLFrkxo+28uLb/NYRcStH48fnD0Vzw==",
|
||||
"bin": {
|
||||
"uuid": "dist/bin/uuid"
|
||||
}
|
||||
},
|
||||
"libraries/overleaf-editor-core": {
|
||||
"version": "1.0.0",
|
||||
"license": "Proprietary",
|
||||
|
|
@ -49452,7 +49481,7 @@
|
|||
"@overleaf/logger": "*",
|
||||
"@overleaf/metrics": "*",
|
||||
"@overleaf/o-error": "*",
|
||||
"aws-sdk": "^2.718.0",
|
||||
"aws-sdk": "^2.1691.0",
|
||||
"chai": "^4.3.6",
|
||||
"chai-as-promised": "^7.1.1",
|
||||
"fast-crc32c": "overleaf/node-fast-crc32c#aae6b2a4c7a7a159395df9cc6c38dfde702d6f51",
|
||||
|
|
@ -49468,6 +49497,23 @@
|
|||
"typescript": "^5.0.4"
|
||||
},
|
||||
"dependencies": {
|
||||
"aws-sdk": {
|
||||
"version": "2.1691.0",
|
||||
"resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.1691.0.tgz",
|
||||
"integrity": "sha512-/F2YC+DlsY3UBM2Bdnh5RLHOPNibS/+IcjUuhP8XuctyrN+MlL+fWDAiela32LTDk7hMy4rx8MTgvbJ+0blO5g==",
|
||||
"requires": {
|
||||
"buffer": "4.9.2",
|
||||
"events": "1.1.1",
|
||||
"ieee754": "1.1.13",
|
||||
"jmespath": "0.16.0",
|
||||
"querystring": "0.2.0",
|
||||
"sax": "1.2.1",
|
||||
"url": "0.10.3",
|
||||
"util": "^0.12.4",
|
||||
"uuid": "8.0.0",
|
||||
"xml2js": "0.6.2"
|
||||
}
|
||||
},
|
||||
"gcp-metadata": {
|
||||
"version": "5.3.0",
|
||||
"resolved": "https://registry.npmjs.org/gcp-metadata/-/gcp-metadata-5.3.0.tgz",
|
||||
|
|
@ -49490,6 +49536,11 @@
|
|||
"bson": "^6.1.0",
|
||||
"mongodb-connection-string-url": "^2.6.0"
|
||||
}
|
||||
},
|
||||
"uuid": {
|
||||
"version": "8.0.0",
|
||||
"resolved": "https://registry.npmjs.org/uuid/-/uuid-8.0.0.tgz",
|
||||
"integrity": "sha512-jOXGuXZAWdsTH7eZLtyXMqUb9EcWMGZNbL9YcGBJl4MH4nrxHmZJhEHvyLFrkxo+28uLb/NYRcStH48fnD0Vzw=="
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -32,6 +32,10 @@ process.on('unhandledRejection', e => {
|
|||
// store settings for multiple backends, so that we can test each one.
|
||||
// fs will always be available - add others if they are configured
|
||||
const { BackendSettings, s3Config } = require('./TestConfig')
|
||||
const {
|
||||
AlreadyWrittenError,
|
||||
NotImplementedError,
|
||||
} = require('@overleaf/object-persistor/src/Errors')
|
||||
|
||||
describe('Filestore', function () {
|
||||
this.timeout(1000 * 10)
|
||||
|
|
@ -273,6 +277,40 @@ describe('Filestore', function () {
|
|||
expect(body).to.equal(newContent)
|
||||
})
|
||||
|
||||
describe('IfNoneMatch', function () {
|
||||
if (backendSettings.backend === 'fs') {
|
||||
it('should refuse to handle IfNoneMatch', async function () {
|
||||
await expect(
|
||||
app.persistor.sendStream(
|
||||
Settings.filestore.stores.user_files,
|
||||
`${projectId}/${fileId}`,
|
||||
fs.createReadStream(localFileReadPath),
|
||||
{ ifNoneMatch: '*' }
|
||||
)
|
||||
).to.be.rejectedWith(NotImplementedError)
|
||||
})
|
||||
} else {
|
||||
it('should reject sendStream on the same key with IfNoneMatch', async function () {
|
||||
await expect(
|
||||
app.persistor.sendStream(
|
||||
Settings.filestore.stores.user_files,
|
||||
`${projectId}/${fileId}`,
|
||||
fs.createReadStream(localFileReadPath),
|
||||
{ ifNoneMatch: '*' }
|
||||
)
|
||||
).to.be.rejectedWith(AlreadyWrittenError)
|
||||
})
|
||||
it('should allow sendStream on a different key with IfNoneMatch', async function () {
|
||||
await app.persistor.sendStream(
|
||||
Settings.filestore.stores.user_files,
|
||||
`${projectId}/${fileId}-other`,
|
||||
fs.createReadStream(localFileReadPath),
|
||||
{ ifNoneMatch: '*' }
|
||||
)
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
if (backendSettings.backend !== 'fs') {
|
||||
it('should record an egress metric for the upload', async function () {
|
||||
const metric = await TestHelper.getMetric(
|
||||
|
|
|
|||
Loading…
Reference in a new issue