From 96c0089aac4a6d693f29e25e48e14d871c7ae612 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Tue, 7 Jan 2025 12:06:15 +0800 Subject: [PATCH 1/3] mirage: Fix "version not found" errors to 404 status code --- mirage/route-handlers/crates.js | 25 +++++++++++++++---- tests/mirage/crates/versions/authors-test.js | 6 ++--- .../crates/versions/dependencies-test.js | 6 ++--- .../mirage/crates/versions/downloads-test.js | 6 ++--- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/mirage/route-handlers/crates.js b/mirage/route-handlers/crates.js index f014a47607f..dda65d492e2 100644 --- a/mirage/route-handlers/crates.js +++ b/mirage/route-handlers/crates.js @@ -174,7 +174,12 @@ export function register(server) { let num = request.params.version_num; let version = schema.versions.findBy({ crateId: crate.id, num }); - if (!version) return { errors: [{ detail: `crate \`${crate.name}\` does not have a version \`${num}\`` }] }; + if (!version) + return new Response( + 404, + {}, + { errors: [{ detail: `crate \`${crate.name}\` does not have a version \`${num}\`` }] }, + ); return { meta: { names: [] }, users: [] }; }); @@ -186,7 +191,12 @@ export function register(server) { let num = request.params.version_num; let version = schema.versions.findBy({ crateId: crate.id, num }); - if (!version) return { errors: [{ detail: `crate \`${crate.name}\` does not have a version \`${num}\`` }] }; + if (!version) + return new Response( + 404, + {}, + { errors: [{ detail: `crate \`${crate.name}\` does not have a version \`${num}\`` }] }, + ); return schema.dependencies.where({ versionId: version.id }); }); @@ -196,9 +206,14 @@ export function register(server) { let crate = schema.crates.findBy({ name }); if (!crate) return notFound(); - let versionNum = request.params.version_num; - let version = schema.versions.findBy({ crateId: crate.id, num: versionNum }); - if (!version) return { errors: [{ detail: `crate \`${crate.name}\` does not have a version \`${versionNum}\`` }] }; + let num = request.params.version_num; + let version = schema.versions.findBy({ crateId: crate.id, num }); + if (!version) + return new Response( + 404, + {}, + { errors: [{ detail: `crate \`${crate.name}\` does not have a version \`${num}\`` }] }, + ); return schema.versionDownloads.where({ versionId: version.id }); }); diff --git a/tests/mirage/crates/versions/authors-test.js b/tests/mirage/crates/versions/authors-test.js index c9f1f3fba81..36ce69f19aa 100644 --- a/tests/mirage/crates/versions/authors-test.js +++ b/tests/mirage/crates/versions/authors-test.js @@ -15,13 +15,11 @@ module('Mirage | GET /api/v1/crates/:id/:version/authors', function (hooks) { assert.deepEqual(await response.json(), { errors: [{ detail: 'Not Found' }] }); }); - test('returns 200 for unknown versions', async function (assert) { + test('returns 404 for unknown versions', async function (assert) { this.server.create('crate', { name: 'rand' }); let response = await fetch('/api/v1/crates/rand/1.0.0/authors'); - // we should probably return 404 for this, but the production API - // currently doesn't do this either - assert.strictEqual(response.status, 200); + assert.strictEqual(response.status, 404); assert.deepEqual(await response.json(), { errors: [{ detail: 'crate `rand` does not have a version `1.0.0`' }] }); }); diff --git a/tests/mirage/crates/versions/dependencies-test.js b/tests/mirage/crates/versions/dependencies-test.js index 83a03cc69d4..1c23bd947c6 100644 --- a/tests/mirage/crates/versions/dependencies-test.js +++ b/tests/mirage/crates/versions/dependencies-test.js @@ -15,13 +15,11 @@ module('Mirage | GET /api/v1/crates/:id/:version/dependencies', function (hooks) assert.deepEqual(await response.json(), { errors: [{ detail: 'Not Found' }] }); }); - test('returns 200 for unknown versions', async function (assert) { + test('returns 404 for unknown versions', async function (assert) { this.server.create('crate', { name: 'rand' }); let response = await fetch('/api/v1/crates/rand/1.0.0/dependencies'); - // we should probably return 404 for this, but the production API - // currently doesn't do this either - assert.strictEqual(response.status, 200); + assert.strictEqual(response.status, 404); assert.deepEqual(await response.json(), { errors: [{ detail: 'crate `rand` does not have a version `1.0.0`' }] }); }); diff --git a/tests/mirage/crates/versions/downloads-test.js b/tests/mirage/crates/versions/downloads-test.js index 09f464905bc..9d28c3bf8ef 100644 --- a/tests/mirage/crates/versions/downloads-test.js +++ b/tests/mirage/crates/versions/downloads-test.js @@ -15,13 +15,11 @@ module('Mirage | GET /api/v1/crates/:id/:version/downloads', function (hooks) { assert.deepEqual(await response.json(), { errors: [{ detail: 'Not Found' }] }); }); - test('returns 200 for unknown versions', async function (assert) { + test('returns 404 for unknown versions', async function (assert) { this.server.create('crate', { name: 'rand' }); let response = await fetch('/api/v1/crates/rand/1.0.0/downloads'); - // we should probably return 404 for this, but the production API - // currently doesn't do this either - assert.strictEqual(response.status, 200); + assert.strictEqual(response.status, 404); assert.deepEqual(await response.json(), { errors: [{ detail: 'crate `rand` does not have a version `1.0.0`' }] }); }); From bb9dc89aeb4d9b6f842be35d2f88877a1d5d8862 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Tue, 7 Jan 2025 12:10:46 +0800 Subject: [PATCH 2/3] mirage: Add `GET /api/v1/crates/:name/:version` endpoint --- mirage/route-handlers/crates.js | 17 ++++++ .../mirage/crates/versions/get-by-num-test.js | 57 +++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 tests/mirage/crates/versions/get-by-num-test.js diff --git a/mirage/route-handlers/crates.js b/mirage/route-handlers/crates.js index dda65d492e2..6b976a84b8d 100644 --- a/mirage/route-handlers/crates.js +++ b/mirage/route-handlers/crates.js @@ -354,6 +354,23 @@ export function register(server) { return { ok: true, msg: 'owners successfully removed' }; }); + server.get('/api/v1/crates/:name/:version', function (schema, request) { + let { name } = request.params; + let crate = schema.crates.findBy({ name }); + if (!crate) return notFound(); + + let num = request.params.version; + let version = schema.versions.findBy({ crateId: crate.id, num }); + if (!version) { + return new Response( + 404, + {}, + { errors: [{ detail: `crate \`${crate.name}\` does not have a version \`${num}\`` }] }, + ); + } + return this.serialize(version); + }); + server.patch('/api/v1/crates/:name/:version', function (schema, request) { let { user } = getSession(schema); if (!user) { diff --git a/tests/mirage/crates/versions/get-by-num-test.js b/tests/mirage/crates/versions/get-by-num-test.js new file mode 100644 index 00000000000..27496883322 --- /dev/null +++ b/tests/mirage/crates/versions/get-by-num-test.js @@ -0,0 +1,57 @@ +import { module, test } from 'qunit'; + +import fetch from 'fetch'; + +import { setupTest } from '../../../helpers'; +import setupMirage from '../../../helpers/setup-mirage'; + +module('Mirage | GET /api/v1/crates/:name/:version', function (hooks) { + setupTest(hooks); + setupMirage(hooks); + + test('returns 404 for unknown crates', async function (assert) { + let response = await fetch('/api/v1/crates/foo/1.0.0-beta.1'); + assert.strictEqual(response.status, 404); + assert.deepEqual(await response.json(), { errors: [{ detail: 'Not Found' }] }); + }); + + test('returns 404 for unknown versions', async function (assert) { + let crate = this.server.create('crate', { name: 'rand' }); + this.server.create('version', { crate, num: '1.0.0-alpha.1' }); + let response = await fetch('/api/v1/crates/rand/1.0.0-beta.1'); + assert.strictEqual(response.status, 404); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'crate `rand` does not have a version `1.0.0-beta.1`' }], + }); + }); + + test('returns a version object for known version', async function (assert) { + let crate = this.server.create('crate', { name: 'rand' }); + this.server.create('version', { crate, num: '1.0.0-beta.1' }); + + let response = await fetch('/api/v1/crates/rand/1.0.0-beta.1'); + assert.strictEqual(response.status, 200); + assert.deepEqual(await response.json(), { + version: { + crate: 'rand', + crate_size: 0, + created_at: '2010-06-16T21:30:45Z', + dl_path: '/api/v1/crates/rand/1.0.0-beta.1/download', + downloads: 0, + id: '1', + license: 'MIT/Apache-2.0', + links: { + dependencies: '/api/v1/crates/rand/1.0.0-beta.1/dependencies', + version_downloads: '/api/v1/crates/rand/1.0.0-beta.1/downloads', + }, + num: '1.0.0-beta.1', + published_by: null, + readme_path: '/api/v1/crates/rand/1.0.0-beta.1/readme', + rust_version: null, + updated_at: '2017-02-24T12:34:56Z', + yank_message: null, + yanked: false, + }, + }); + }); +}); From e20b90355b46d04bdc6efd979be98c53cec097a1 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Tue, 7 Jan 2025 12:21:02 +0800 Subject: [PATCH 3/3] mirage: Reword for consistency --- mirage/route-handlers/crates.js | 28 +++++++++---------- tests/mirage/crates/versions/authors-test.js | 2 +- .../crates/versions/dependencies-test.js | 2 +- .../mirage/crates/versions/downloads-test.js | 2 +- tests/mirage/crates/versions/list-test.js | 2 +- tests/mirage/crates/versions/patch-test.js | 2 +- tests/mirage/crates/versions/readme-test.js | 2 +- .../crates/versions/yank/unyank-test.js | 2 +- .../mirage/crates/versions/yank/yank-test.js | 2 +- 9 files changed, 22 insertions(+), 22 deletions(-) diff --git a/mirage/route-handlers/crates.js b/mirage/route-handlers/crates.js index 6b976a84b8d..d432e01f02c 100644 --- a/mirage/route-handlers/crates.js +++ b/mirage/route-handlers/crates.js @@ -167,12 +167,12 @@ export function register(server) { return resp; }); - server.get('/api/v1/crates/:name/:version_num/authors', (schema, request) => { + server.get('/api/v1/crates/:name/:version/authors', (schema, request) => { let { name } = request.params; let crate = schema.crates.findBy({ name }); if (!crate) return notFound(); - let num = request.params.version_num; + let num = request.params.version; let version = schema.versions.findBy({ crateId: crate.id, num }); if (!version) return new Response( @@ -184,12 +184,12 @@ export function register(server) { return { meta: { names: [] }, users: [] }; }); - server.get('/api/v1/crates/:name/:version_num/dependencies', (schema, request) => { + server.get('/api/v1/crates/:name/:version/dependencies', (schema, request) => { let { name } = request.params; let crate = schema.crates.findBy({ name }); if (!crate) return notFound(); - let num = request.params.version_num; + let num = request.params.version; let version = schema.versions.findBy({ crateId: crate.id, num }); if (!version) return new Response( @@ -201,12 +201,12 @@ export function register(server) { return schema.dependencies.where({ versionId: version.id }); }); - server.get('/api/v1/crates/:name/:version_num/downloads', function (schema, request) { + server.get('/api/v1/crates/:name/:version/downloads', function (schema, request) { let { name } = request.params; let crate = schema.crates.findBy({ name }); if (!crate) return notFound(); - let num = request.params.version_num; + let num = request.params.version; let version = schema.versions.findBy({ crateId: crate.id, num }); if (!version) return new Response( @@ -377,13 +377,13 @@ export function register(server) { return new Response(403, {}, { errors: [{ detail: 'must be logged in to perform that action' }] }); } - const { name, version: versionNum } = request.params; + const { name, version: num } = request.params; const crate = schema.crates.findBy({ name }); if (!crate) { return notFound(); } - const version = schema.versions.findBy({ crateId: crate.id, num: versionNum }); + const version = schema.versions.findBy({ crateId: crate.id, num }); if (!version) { return notFound(); } @@ -403,13 +403,13 @@ export function register(server) { return new Response(403, {}, { errors: [{ detail: 'must be logged in to perform that action' }] }); } - const { name, version: versionNum } = request.params; + const { name, version: num } = request.params; const crate = schema.crates.findBy({ name }); if (!crate) { return notFound(); } - const version = schema.versions.findBy({ crateId: crate.id, num: versionNum }); + const version = schema.versions.findBy({ crateId: crate.id, num }); if (!version) { return notFound(); } @@ -425,13 +425,13 @@ export function register(server) { return new Response(403, {}, { errors: [{ detail: 'must be logged in to perform that action' }] }); } - const { name, version: versionNum } = request.params; + const { name, version: num } = request.params; const crate = schema.crates.findBy({ name }); if (!crate) { return notFound(); } - const version = schema.versions.findBy({ crateId: crate.id, num: versionNum }); + const version = schema.versions.findBy({ crateId: crate.id, num }); if (!version) { return notFound(); } @@ -442,13 +442,13 @@ export function register(server) { }); server.get('/api/v1/crates/:name/:version/readme', (schema, request) => { - const { name, version: versionNum } = request.params; + const { name, version: num } = request.params; const crate = schema.crates.findBy({ name }); if (!crate) { return new Response(403, { 'Content-Type': 'text/html' }, ''); } - const version = schema.versions.findBy({ crateId: crate.id, num: versionNum }); + const version = schema.versions.findBy({ crateId: crate.id, num }); if (!version || !version.readme) { return new Response(403, { 'Content-Type': 'text/html' }, ''); } diff --git a/tests/mirage/crates/versions/authors-test.js b/tests/mirage/crates/versions/authors-test.js index 36ce69f19aa..8f8bbab45bb 100644 --- a/tests/mirage/crates/versions/authors-test.js +++ b/tests/mirage/crates/versions/authors-test.js @@ -5,7 +5,7 @@ import fetch from 'fetch'; import { setupTest } from '../../../helpers'; import setupMirage from '../../../helpers/setup-mirage'; -module('Mirage | GET /api/v1/crates/:id/:version/authors', function (hooks) { +module('Mirage | GET /api/v1/crates/:name/:version/authors', function (hooks) { setupTest(hooks); setupMirage(hooks); diff --git a/tests/mirage/crates/versions/dependencies-test.js b/tests/mirage/crates/versions/dependencies-test.js index 1c23bd947c6..823663ee2ea 100644 --- a/tests/mirage/crates/versions/dependencies-test.js +++ b/tests/mirage/crates/versions/dependencies-test.js @@ -5,7 +5,7 @@ import fetch from 'fetch'; import { setupTest } from '../../../helpers'; import setupMirage from '../../../helpers/setup-mirage'; -module('Mirage | GET /api/v1/crates/:id/:version/dependencies', function (hooks) { +module('Mirage | GET /api/v1/crates/:name/:version/dependencies', function (hooks) { setupTest(hooks); setupMirage(hooks); diff --git a/tests/mirage/crates/versions/downloads-test.js b/tests/mirage/crates/versions/downloads-test.js index 9d28c3bf8ef..59ea3125726 100644 --- a/tests/mirage/crates/versions/downloads-test.js +++ b/tests/mirage/crates/versions/downloads-test.js @@ -5,7 +5,7 @@ import fetch from 'fetch'; import { setupTest } from '../../../helpers'; import setupMirage from '../../../helpers/setup-mirage'; -module('Mirage | GET /api/v1/crates/:id/:version/downloads', function (hooks) { +module('Mirage | GET /api/v1/crates/:name/:version/downloads', function (hooks) { setupTest(hooks); setupMirage(hooks); diff --git a/tests/mirage/crates/versions/list-test.js b/tests/mirage/crates/versions/list-test.js index d4a4459849c..bae2ce112bf 100644 --- a/tests/mirage/crates/versions/list-test.js +++ b/tests/mirage/crates/versions/list-test.js @@ -5,7 +5,7 @@ import fetch from 'fetch'; import { setupTest } from '../../../helpers'; import setupMirage from '../../../helpers/setup-mirage'; -module('Mirage | GET /api/v1/crates/:id/versions', function (hooks) { +module('Mirage | GET /api/v1/crates/:name/versions', function (hooks) { setupTest(hooks); setupMirage(hooks); diff --git a/tests/mirage/crates/versions/patch-test.js b/tests/mirage/crates/versions/patch-test.js index e55db8ba625..00472129f38 100644 --- a/tests/mirage/crates/versions/patch-test.js +++ b/tests/mirage/crates/versions/patch-test.js @@ -18,7 +18,7 @@ const UNYANK_BODY = JSON.stringify({ }, }); -module('Mirage | PATCH /api/v1/crates/:crate/:version', function (hooks) { +module('Mirage | PATCH /api/v1/crates/:name/:version', function (hooks) { setupTest(hooks); setupMirage(hooks); diff --git a/tests/mirage/crates/versions/readme-test.js b/tests/mirage/crates/versions/readme-test.js index 95af3303c46..03746524f95 100644 --- a/tests/mirage/crates/versions/readme-test.js +++ b/tests/mirage/crates/versions/readme-test.js @@ -5,7 +5,7 @@ import fetch from 'fetch'; import { setupTest } from '../../../helpers'; import setupMirage from '../../../helpers/setup-mirage'; -module('Mirage | GET /api/v1/crates/:id/:version/readme', function (hooks) { +module('Mirage | GET /api/v1/crates/:name/:version/readme', function (hooks) { setupTest(hooks); setupMirage(hooks); diff --git a/tests/mirage/crates/versions/yank/unyank-test.js b/tests/mirage/crates/versions/yank/unyank-test.js index 5db7a5f504b..b5fa969a79e 100644 --- a/tests/mirage/crates/versions/yank/unyank-test.js +++ b/tests/mirage/crates/versions/yank/unyank-test.js @@ -5,7 +5,7 @@ import fetch from 'fetch'; import { setupTest } from '../../../../helpers'; import setupMirage from '../../../../helpers/setup-mirage'; -module('Mirage | PUT /api/v1/crates/:crateId/unyank', function (hooks) { +module('Mirage | PUT /api/v1/crates/:name/unyank', function (hooks) { setupTest(hooks); setupMirage(hooks); diff --git a/tests/mirage/crates/versions/yank/yank-test.js b/tests/mirage/crates/versions/yank/yank-test.js index 8dc79b03691..a08af7055c2 100644 --- a/tests/mirage/crates/versions/yank/yank-test.js +++ b/tests/mirage/crates/versions/yank/yank-test.js @@ -5,7 +5,7 @@ import fetch from 'fetch'; import { setupTest } from '../../../../helpers'; import setupMirage from '../../../../helpers/setup-mirage'; -module('Mirage | DELETE /api/v1/crates/:crateId/yank', function (hooks) { +module('Mirage | DELETE /api/v1/crates/:name/yank', function (hooks) { setupTest(hooks); setupMirage(hooks);