From 208b0e4adbaa4bc252cc7d4953fd1c62c64b4a03 Mon Sep 17 00:00:00 2001 From: Jonathan Gramain Date: Wed, 13 May 2026 16:18:59 -0700 Subject: [PATCH 1/2] SPRXCLT-23: support query params and conditional PUT/DELETE in putEmptyObject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes to lib/sproxyd.js: - _createRequestHeader: append query string from params.query to request path - putEmptyObject: accept params object (params.headers, params.query); return x-scal-version from PUT response as second callback argument (not the key) - putEmptyObject: fix non-streaming error handling to propagate expected errors (err.isExpected) rather than always wrapping in InternalError - deleteWithParams: new method — version-conditional DELETE via params.query.version; avoids changing the existing delete() signature for backward compatibility Changes to tests/unit/sproxyd.js: - test server: support conditional PUT/DELETE (version param, If-None-Match: *), track per-key versions, return x-scal-version in PUT/HEAD responses - update existing putEmptyObject and conditional-PUT tests for new signatures - add three deleteWithParams tests (matching version, wrong version, absent key) Co-Authored-By: Claude Sonnet 4.6 --- lib/sproxyd.js | 56 ++++++++++--- tests/unit/sproxyd.js | 182 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 204 insertions(+), 34 deletions(-) diff --git a/lib/sproxyd.js b/lib/sproxyd.js index 67300ea..62ff1bd 100644 --- a/lib/sproxyd.js +++ b/lib/sproxyd.js @@ -211,11 +211,13 @@ class SproxydClient { if (params && params.range) { reqHeaders['Range'] = `bytes=${params.range[0]}-${params.range[1]}`; } + const query = params && params.query + ? `?${new URLSearchParams(params.query).toString()}` : ''; return { hostname: currentBootstrap[0], port: currentBootstrap[1], method, - path: `${this.path}${key}`, + path: `${this.path}${key}${query}`, headers: reqHeaders, agent: this.httpAgent, }; @@ -378,8 +380,10 @@ class SproxydClient { : contentType; const request = _createRequest(req, log, (err, response) => { if (err) { - if (err.code === 404) { - log.debug('sproxyd responded with error 404 not found'); + if (err.isExpected) { + log.debug('sproxyd responded with expected error', { + statusCode: err.code, + }); return callback(err); } const errorProcessed = err; @@ -425,23 +429,30 @@ class SproxydClient { } /** - * This sends a PUT request to sproxyd without data. - * @param {String} keyScheme - sproxyd key for put the metadata - * @param {String} metadata - metadata to put in the object + * This sends an empty-body PUT request to sproxyd. + * @param {String} keyScheme - sproxyd key + * @param {String} metadata - metadata to set in x-scal-usermd header + * @param {Object} params - optional parameters + * @param {Object} [params.headers] - additional request headers (e.g. If-None-Match) + * @param {Object} [params.query] - query parameters as key-value pairs (e.g. { version: 64 }) * @param {String} reqUids - The serialized request id * @param {SproxydClient~putCallback} callback - callback * @returns {undefined} */ - putEmptyObject(keyScheme, metadata, reqUids, callback) { + putEmptyObject(keyScheme, metadata, params, reqUids, callback) { const log = this.createLogger(reqUids); - const params = { headers: {} }; - params.headers['x-scal-usermd'] = metadata; - this._failover('PUT', null, 0, keyScheme, 0, log, (err, key) => { + const reqParams = Object.assign({}, params, { + headers: Object.assign({}, params && params.headers), + }); + reqParams.headers['x-scal-usermd'] = metadata; + this._failover('PUT', null, 0, keyScheme, 0, log, (err, response) => { if (err) { return callback(err); } - return callback(null, key); - }, params); + const version = response && response.headers + ? response.headers['x-scal-version'] : undefined; + return callback(null, version); + }, reqParams); } /** @@ -528,6 +539,27 @@ class SproxydClient { this._failover('DELETE', null, 0, key, 0, log, callback); } + /** + * This sends a DELETE request to sproxyd with optional query parameters. + * @param {String} key - The key associated to the values + * @param {Object} params - optional parameters + * @param {Object} [params.query] - query parameters as key-value pairs (e.g. { version: 64 }) + * @param {String} reqUids - The serialized request id + * @param {SproxydClient~deleteCallback} callback - callback + * @returns {undefined} + */ + deleteWithParams(key, params, reqUids, callback) { + assert.strictEqual(typeof key, 'string'); + assert.strictEqual(key.length, 40); + const log = this.createLogger(reqUids); + this._failover('DELETE', null, 0, key, 0, log, err => { + if (err) { + return callback(err); + } + return callback(null); + }, params); + } + /** * This sends a BATCH DELETE request to sproxyd. * @param {Object} list - object containing a list of keys to delete diff --git a/tests/unit/sproxyd.js b/tests/unit/sproxyd.js index 0d9d780..068ee24 100644 --- a/tests/unit/sproxyd.js +++ b/tests/unit/sproxyd.js @@ -16,6 +16,7 @@ const upload = cryptoLib.randomBytes(9000); let savedKey; let server; const md = {}; +const versions = {}; let mdHex; let expectedRequestHeaders; let notExpectedRequestHeaders; @@ -70,7 +71,7 @@ function checkKeyContent(client, key, body, reqUid, callback) { }); } -function makeResponse(res, code, message, data, md) { +function makeResponse(res, code, message, data, md, version) { /* eslint-disable no-param-reassign */ res.statusCode = code; res.statusMessage = message; @@ -81,6 +82,9 @@ function makeResponse(res, code, message, data, md) { if (md) { res.setHeader('x-scal-usermd', md); } + if (version !== undefined) { + res.setHeader('x-scal-version', version.toString()); + } res.end(); } @@ -89,7 +93,9 @@ function consumeAndMakeResponse(req, res, code, message, data, md) { } function handler(req, res) { - const key = req.url.slice(-40); + const [urlPath, queryString] = req.url.split('?'); + const key = urlPath.slice(-40); + const query = new URLSearchParams(queryString || ''); if (expectedRequestHeaders) { Object.keys(expectedRequestHeaders).forEach(header => { assert.strictEqual(req.headers[header], @@ -103,21 +109,38 @@ function handler(req, res) { } if (req.url === '/proxy/arc/.conf' && req.method === 'GET') { consumeAndMakeResponse(req, res, 200, 'OK'); - } else if (!req.url.startsWith('/proxy/arc')) { + } else if (!urlPath.startsWith('/proxy/arc')) { consumeAndMakeResponse(req, res, 404, 'NoSuchPath'); } else if (req.method === 'PUT') { - if (server[key]) { - consumeAndMakeResponse(req, res, 404, 'AlreadyExists'); - } else { - server[key] = Buffer.alloc(0); - if (req.headers['x-scal-usermd']) { - md[key] = req.headers['x-scal-usermd']; + const versionParam = query.get('version'); + if (versionParam !== null) { + // conditional PUT: 422 if key absent, 412 if version mismatch + if (server[key] === undefined) { + consumeAndMakeResponse(req, res, 422, 'UnprocessableEntity'); + return; + } + const expectedVersion = parseInt(versionParam, 10); + if (versions[key] !== expectedVersion) { + consumeAndMakeResponse(req, res, 412, 'PreconditionFailed'); + return; } - req.on('data', data => { - server[key] = Buffer.concat([server[key], data]); - }) - .on('end', () => makeResponse(res, 200, 'OK')); + } else if (req.headers['if-none-match'] === '*') { + // fresh create: only succeed if key does not exist + if (server[key] !== undefined) { + consumeAndMakeResponse(req, res, 412, 'PreconditionFailed'); + return; + } + } + server[key] = Buffer.alloc(0); + if (req.headers['x-scal-usermd']) { + md[key] = req.headers['x-scal-usermd']; } + versions[key] = (versions[key] || 0) + 64; + req.on('data', data => { + server[key] = Buffer.concat([server[key], data]); + }) + .on('end', () => makeResponse(res, 200, 'OK', null, null, + versions[key])); } else if (req.method === 'GET') { if (!server[key]) { consumeAndMakeResponse(req, res, 404, 'NoSuchPath'); @@ -127,18 +150,31 @@ function handler(req, res) { } else if (req.method === 'DELETE') { if (key === lockedObjectKey) { consumeAndMakeResponse(req, res, 423, 'Locked'); + return; + } + const deleteVersionParam = query.get('version'); + if (deleteVersionParam !== null) { + if (server[key] === undefined) { + consumeAndMakeResponse(req, res, 422, 'UnprocessableEntity'); + return; + } + const expectedVersion = parseInt(deleteVersionParam, 10); + if (versions[key] !== expectedVersion) { + consumeAndMakeResponse(req, res, 412, 'PreconditionFailed'); + return; + } } else if (!server[key]) { consumeAndMakeResponse(req, res, 404, 'NoSuchPath'); - } else { - delete server[key]; - if (md[key]) { - delete md[key]; - } - consumeAndMakeResponse(req, res, 200, 'OK'); + return; } + delete server[key]; + delete md[key]; + delete versions[key]; + consumeAndMakeResponse(req, res, 200, 'OK'); } else if (req.method === 'HEAD') { if (server[key]) { - consumeAndMakeResponse(req, res, 200, 'OK', null, md[key]); + req.resume().on('end', () => + makeResponse(res, 200, 'OK', null, md[key], versions[key])); } else { consumeAndMakeResponse(req, res, 404, 'NoSuchPath'); } @@ -272,8 +308,10 @@ const clientImmutableWithFailover = new Sproxy({ it('should put an empty object via sproxyd', done => { savedKey = generateKey(); mdHex = generateMD(); - client.putEmptyObject(savedKey, mdHex, reqUid, err => { - done(err); + client.putEmptyObject(savedKey, mdHex, {}, reqUid, (err, version) => { + assert.strictEqual(err, null); + assert(version, 'expected version from putEmptyObject'); + done(); }); }); @@ -302,6 +340,106 @@ const clientImmutableWithFailover = new Sproxy({ }); }); + it('should put an empty object with If-None-Match on a fresh key', done => { + const key = generateKey(); + const meta = generateMD(); + client.putEmptyObject(key, meta, { + headers: { 'If-None-Match': '*' }, + }, reqUid, err => { + done(err); + }); + }); + + it('should fail put with If-None-Match when key already exists', done => { + const key = generateKey(); + const meta = generateMD(); + async.series([ + next => client.putEmptyObject(key, meta, {}, reqUid, next), + next => client.putEmptyObject(key, meta, { + headers: { 'If-None-Match': '*' }, + }, reqUid, err => { + assert(err, 'expected error for duplicate key with If-None-Match'); + assert.strictEqual(err.code, 412); + next(); + }), + ], done); + }); + + it('should succeed conditional PUT with matching version', done => { + const key = generateKey(); + const meta = generateMD(); + const meta2 = generateMD(); + async.waterfall([ + next => client.putEmptyObject(key, meta, {}, reqUid, next), + (version, next) => { + assert(version, 'expected version from putEmptyObject'); + client.putEmptyObject(key, meta2, { + query: { version }, + }, reqUid, next); + }, + ], done); + }); + + it('should fail conditional PUT with wrong version', done => { + const key = generateKey(); + const meta = generateMD(); + async.series([ + next => client.putEmptyObject(key, meta, {}, reqUid, next), + next => client.putEmptyObject(key, meta, { + query: { version: 0 }, + }, reqUid, err => { + assert(err, 'expected error for wrong version'); + assert.strictEqual(err.code, 412); + next(); + }), + ], done); + }); + + it('should return 422 for conditional PUT on non-existent key', done => { + const key = generateKey(); + const meta = generateMD(); + client.putEmptyObject(key, meta, { + query: { version: 64 }, + }, reqUid, err => { + assert(err, 'expected error for version on missing key'); + assert.strictEqual(err.code, 422); + done(); + }); + }); + + it('should delete an object conditionally via deleteWithParams', done => { + const key = generateKey(); + const meta = generateMD(); + async.waterfall([ + next => client.putEmptyObject(key, meta, {}, reqUid, next), + (version, next) => client.deleteWithParams(key, + { query: { version } }, reqUid, next), + ], done); + }); + + it('should fail deleteWithParams with wrong version', done => { + const key = generateKey(); + const meta = generateMD(); + async.series([ + next => client.putEmptyObject(key, meta, {}, reqUid, next), + next => client.deleteWithParams(key, + { query: { version: 0 } }, reqUid, err => { + assert(err, 'expected error for wrong version'); + assert.strictEqual(err.code, 412); + next(); + }), + ], done); + }); + + it('should return 422 for deleteWithParams on non-existent key', done => { + const key = generateKey(); + client.deleteWithParams(key, { query: { version: 64 } }, reqUid, err => { + assert(err, 'expected error for version on missing key'); + assert.strictEqual(err.code, 422); + done(); + }); + }); + it('should abort an unfinished request', done => { const upStream = new stream.PassThrough(); upStream.write(upload.slice(0, upload.length - 10)); From 9aae3adb10d029c6c4133094c218c4999c5f7941 Mon Sep 17 00:00:00 2001 From: Jonathan Gramain Date: Wed, 13 May 2026 17:48:51 -0700 Subject: [PATCH 2/2] SPRXCLT-23 bump version to 9.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5493a54..ed738cf 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "engines": { "node": ">=20" }, - "version": "8.2.0", + "version": "9.0.0", "description": "sproxyd client", "main": "index.js", "scripts": {