diff --git a/packages/api/src/pins.js b/packages/api/src/pins.js index e537c9b00e..41c48acbf9 100644 --- a/packages/api/src/pins.js +++ b/packages/api/src/pins.js @@ -187,7 +187,7 @@ export async function pinsGet (request, env, ctx) { /** * Transform a PinRequest into a PinStatus * - * @param { Object } pinRequest + * @param { import('../../db/db-client-types.js').PsaPinRequestUpsertOutput } pinRequest * @returns { PsaPinStatusResponse } */ export function toPinStatusResponse (pinRequest) { @@ -197,9 +197,10 @@ export function toPinStatusResponse (pinRequest) { created: pinRequest.created, pin: { cid: pinRequest.sourceCid, - ...pinRequest + ...pinRequest.name && { name: pinRequest.name }, + ...pinRequest.origins && { origins: pinRequest.origins }, + ...pinRequest.meta && { meta: pinRequest.meta } }, - // TODO(https://github.com/web3-storage/web3.storage/issues/792) delegates: [] } } diff --git a/packages/api/src/utils/psa.js b/packages/api/src/utils/psa.js index 5ea35f60ff..e2333ed006 100644 --- a/packages/api/src/utils/psa.js +++ b/packages/api/src/utils/psa.js @@ -74,7 +74,7 @@ export const MAX_PIN_LISTING_LIMIT = 1000 // Validation schemas const listPinsValidator = new Validator({ type: 'object', - required: ['status'], + required: [], properties: { name: { type: 'string', maxLength: 255 }, after: { type: 'string', format: 'date-time' }, @@ -234,7 +234,7 @@ export function validateSearchParams (queryString) { if (result.valid) { // Map statuses for DB compatibility. - opts.statuses = psaStatusesToDBStatuses(opts.status) + opts.statuses = opts.status && psaStatusesToDBStatuses(opts.status) data = opts } else { error = parseValidatorErrors(result.errors) diff --git a/packages/api/test/pin.spec.js b/packages/api/test/pin.spec.js index aaa2ec5d65..660168e529 100644 --- a/packages/api/test/pin.spec.js +++ b/packages/api/test/pin.spec.js @@ -11,6 +11,9 @@ import { } from '../src/utils/psa.js' import { PSAErrorResourceNotFound, PSAErrorInvalidData, PSAErrorRequiredData } from '../src/errors.js' +const REQUEST_EXPECTED_PROPERTIES = ['requestid', 'status', 'created', 'delegates', 'info', 'pin'] +const PIN_EXPECTED_PROPERTIES = ['cid', 'name', 'origins', 'meta'] + /** * * @param {import('../../db/postgres/pg-rest-api-types').definitions['pin']['status']} status @@ -41,24 +44,41 @@ const assertCorrectPinResponse = (data) => { assert.ok(Date.parse(data.created), 'created should be valid date string') assert.ok(Array.isArray(data.delegates), 'delegates should be an array') - if (data.info) { + // @ts-ignore https://github.com/microsoft/TypeScript/issues/44253 + if (Object.hasOwn(data, 'info')) { assert.ok(typeof data.info === 'object', 'info should be an object') } assert.ok(typeof data.pin === 'object', 'pin should be an object') assert.ok(typeof data.pin.cid === 'string', 'pin.cid should be an string') - if (data.pin.name) { + // @ts-ignore https://github.com/microsoft/TypeScript/issues/44253 + if (Object.hasOwn(data.pin, 'name')) { assert.ok(typeof data.pin.name === 'string', 'pin.name should be an string') } - if (data.pin.origins) { + // @ts-ignore https://github.com/microsoft/TypeScript/issues/44253 + if (Object.hasOwn(data.pin, 'origins')) { assert.ok(Array.isArray(data.pin.origins), 'pin.origins should be an array') } - if (data.pin.meta) { + // @ts-ignore https://github.com/microsoft/TypeScript/issues/44253 + if (Object.hasOwn(data.pin, 'meta')) { assert.ok(typeof data.pin.meta === 'object', 'pin.meta should be an object') } + + assert.ok(Object.keys(data).every(property => REQUEST_EXPECTED_PROPERTIES.includes(property)), 'request contains not valid properties') + assert.ok(Object.keys(data.pin).every(property => PIN_EXPECTED_PROPERTIES.includes(property)), 'request.pin contains not valid properties') +} + +/** + * + * @param {object} data + */ +const assertCorretPinResponseList = (data) => { + assert.ok(typeof data.count === 'number', 'count should be a number') + data.results.forEach(r => assertCorrectPinResponse(r)) + Object.keys(data).every(property => ['count', 'results'].includes(property)) } /** @@ -121,24 +141,6 @@ describe('Pinning APIs endpoints', () => { assert.strictEqual(error.details, '#/limit: Instance type "number" is invalid. Expected "integer".') }) - it('requires status', async () => { - const url = new URL(`${baseUrl}`).toString() - const res = await fetch( - url, { - method: 'GET', - headers: { - Authorization: `Bearer ${token}`, - 'Content-Type': 'application/json' - } - }) - - assert(res, 'Server responded') - assert.strictEqual(res.status, ERROR_CODE) - const { error } = await res.json() - assert.strictEqual(error.reason, PSAErrorRequiredData.CODE) - assert.strictEqual(error.details, 'Instance does not have required property "status".') - }) - it('validates meta filter is json object', async () => { const opts = new URLSearchParams({ meta: `[ @@ -251,9 +253,7 @@ describe('Pinning APIs endpoints', () => { }) it('returns only successful pins when no filter values are specified', async () => { - const opts = new URLSearchParams({ - status: 'pinned' - }) + const opts = new URLSearchParams({}) const url = new URL(`${baseUrl}?${opts}`).toString() const res = await fetch( url, { @@ -268,6 +268,7 @@ describe('Pinning APIs endpoints', () => { assert(res.ok, 'Server response is ok') const data = await res.json() assert.strictEqual(data.count, 1) + assertCorretPinResponseList(data) }) it('filters pins on CID, for this user', async () => { @@ -294,6 +295,7 @@ describe('Pinning APIs endpoints', () => { assert(res.ok, 'Server response is ok') const data = await res.json() assert.strictEqual(data.count, 2) + assertCorretPinResponseList(data) }) it('filters case sensitive exact match on name', async () => { @@ -315,6 +317,7 @@ describe('Pinning APIs endpoints', () => { assert(res.ok, 'Server response is ok') const data = await res.json() assert.strictEqual(data.count, 1) + assertCorretPinResponseList(data) }) it('filters case insensitive partial match on name', async () => { @@ -337,6 +340,7 @@ describe('Pinning APIs endpoints', () => { assert(res.ok, 'Server response is ok') const data = await res.json() assert.strictEqual(data.count, 4) + assertCorretPinResponseList(data) }) it('filters pins by status', async () => { @@ -359,6 +363,7 @@ describe('Pinning APIs endpoints', () => { assert.strictEqual(data.count, 1) assert.strictEqual(data.results.length, 1) assert.strictEqual(data.results[0].pin.name, 'FailedPinning.doc') + assertCorretPinResponseList(data) }) it('filters pins by multiple statuses', async () => { @@ -379,6 +384,7 @@ describe('Pinning APIs endpoints', () => { assert(res.ok, 'Server response is ok') const data = await res.json() assert.strictEqual(data.count, 5) + assertCorretPinResponseList(data) }) it('filters pins by queued', async () => { @@ -399,6 +405,7 @@ describe('Pinning APIs endpoints', () => { assert(res.ok, 'Server response is ok') const data = await res.json() assert.strictEqual(data.count, 1) + assertCorretPinResponseList(data) }) it('filters pins created before a date', async () => { @@ -421,6 +428,7 @@ describe('Pinning APIs endpoints', () => { const data = await res.json() assert.strictEqual(data.results.length, 1) assert.strictEqual(data.count, 1) + assertCorretPinResponseList(data) }) it('filters pins created after a date', async () => { @@ -443,6 +451,7 @@ describe('Pinning APIs endpoints', () => { const data = await res.json() assert.strictEqual(data.results.length, 2) assert.strictEqual(data.count, 2) + assertCorretPinResponseList(data) }) it('limits the number of pins returned for this user and includes the total', async () => { @@ -465,6 +474,7 @@ describe('Pinning APIs endpoints', () => { const data = await res.json() assert.strictEqual(data.count, 7) assert.strictEqual(data.results.length, 3) + assertCorretPinResponseList(data) }) it('filters pins by meta', async () => { @@ -487,6 +497,7 @@ describe('Pinning APIs endpoints', () => { const data = await res.json() assert.strictEqual(data.count, 1) assert.strictEqual(data.results[0].pin.name, 'Image.jpeg') + assertCorretPinResponseList(data) }) }) @@ -509,10 +520,12 @@ describe('Pinning APIs endpoints', () => { assert(res, 'Server responded') assert(res.ok, 'Server response ok') const data = await res.json() + assertCorrectPinResponse(data) assert.strictEqual(data.pin.cid, cid) - assert.strictEqual(data.pin.name, null) - assert.strictEqual(data.pin.origins, null) - assert.strictEqual(data.pin.meta, null) + console.log(data.pin.cid, cid) + assert.strictEqual(data.pin.name, undefined) + assert.strictEqual(data.pin.origins, undefined) + assert.strictEqual(data.pin.meta, undefined) }) it('requires cid', async () => { @@ -574,6 +587,7 @@ describe('Pinning APIs endpoints', () => { const data = await res.json() assert(res, 'Server responded') assert(res.ok, 'Server response ok') + assertCorrectPinResponse(data) assert.strictEqual(data.pin.cid, cid) assert.deepStrictEqual(data.pin.name, name) assert.deepStrictEqual(data.pin.origins, origins) @@ -971,6 +985,7 @@ describe('Pinning APIs endpoints', () => { assert(replaceResponse.ok, 'Replace request was not successful') assert.strictEqual(replaceResponse.status, 202) const data = await replaceResponse.json() + assertCorrectPinResponse(data) assert.strictEqual(data.pin.cid, newCid) assert.deepStrictEqual(data.pin.name, name) assert.deepStrictEqual(data.pin.origins, origins) diff --git a/packages/api/test/user.spec.js b/packages/api/test/user.spec.js index 9aa67176ae..c2a1e04b40 100644 --- a/packages/api/test/user.spec.js +++ b/packages/api/test/user.spec.js @@ -417,7 +417,7 @@ describe('GET /user/pins', () => { assert(res.ok) const body = await res.json() - assert.equal([...new Set(body.results.map(x => x.pin.authKey))].length, 2) + assert.equal(body.count, 8) }) }) diff --git a/packages/db/index.js b/packages/db/index.js index fda826b8aa..826560e3f1 100644 --- a/packages/db/index.js +++ b/packages/db/index.js @@ -1168,6 +1168,10 @@ export class DBClient { async listPsaPinRequests (authKey, opts = {}) { const match = opts?.match || 'exact' const limit = opts?.limit || 10 + /** + * @type {Array.|undefined} + */ + let statuses let query = this._client .from(psaPinRequestTableName) @@ -1190,12 +1194,17 @@ export class DBClient { query = query.range(rangeFrom, rangeTo - 1) } - if (!opts.cid && !opts.name && !opts.statuses) { - query = query.eq('content.pins.status', 'Pinned') + // If not specified we default to pinned only if no other filters are provided. + // While slightly inconsistent, that's the current expectation. + // This is being discussed in https://github.com/ipfs-shipyard/pinning-service-compliance/issues/245 + if (!opts.cid && !opts.name && !opts.meta && !opts.statuses) { + statuses = ['Pinned'] + } else { + statuses = opts.statuses } - if (opts.statuses) { - query = query.in('content.pins.status', opts.statuses) + if (statuses) { + query = query.in('content.pins.status', statuses) } if (opts.cid) {