From 94abc3f314ebdffe32c8bf03558dd9536cb40dbe Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Fri, 10 Jan 2025 16:29:37 +0100 Subject: [PATCH 1/2] mirage: Allow non-canonical names in `GET /api/v1/crates/:name` route handler --- mirage/route-handlers/crates.js | 7 ++- tests/mirage/crates/get-by-id-test.js | 61 +++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/mirage/route-handlers/crates.js b/mirage/route-handlers/crates.js index 6a57695a71e..e78dc3405a8 100644 --- a/mirage/route-handlers/crates.js +++ b/mirage/route-handlers/crates.js @@ -3,6 +3,10 @@ import { Response } from 'miragejs'; import { getSession } from '../utils/session'; import { compareIsoDates, compareStrings, notFound, pageParams, releaseTracks } from './-utils'; +function toCanonicalName(name) { + return name.toLowerCase().replace(/-/g, '_'); +} + export function list(schema, request) { const { start, end } = pageParams(request); @@ -56,7 +60,8 @@ export function register(server) { server.get('/api/v1/crates/:name', function (schema, request) { let { name } = request.params; - let crate = schema.crates.findBy({ name }); + let canonicalName = toCanonicalName(name); + let crate = schema.crates.all().models.find(it => toCanonicalName(it.name) === canonicalName); if (!crate) return notFound(); let serialized = this.serialize(crate); let includes = request.queryParams?.include ?? ''; diff --git a/tests/mirage/crates/get-by-id-test.js b/tests/mirage/crates/get-by-id-test.js index 712ea30b89a..323532474f1 100644 --- a/tests/mirage/crates/get-by-id-test.js +++ b/tests/mirage/crates/get-by-id-test.js @@ -76,6 +76,67 @@ module('Mirage | GET /api/v1/crates/:id', function (hooks) { }); }); + test('works for non-canonical names', async function (assert) { + let crate = this.server.create('crate', { name: 'foo-bar' }); + this.server.create('version', { crate, num: '1.0.0-beta.1' }); + + let response = await fetch('/api/v1/crates/foo_bar'); + assert.strictEqual(response.status, 200); + assert.deepEqual(await response.json(), { + categories: [], + crate: { + badges: [], + categories: [], + created_at: '2010-06-16T21:30:45Z', + default_version: '1.0.0-beta.1', + description: 'This is the description for the crate called "foo-bar"', + documentation: null, + downloads: 0, + homepage: null, + id: 'foo-bar', + keywords: [], + links: { + owner_team: '/api/v1/crates/foo-bar/owner_team', + owner_user: '/api/v1/crates/foo-bar/owner_user', + reverse_dependencies: '/api/v1/crates/foo-bar/reverse_dependencies', + version_downloads: '/api/v1/crates/foo-bar/downloads', + versions: '/api/v1/crates/foo-bar/versions', + }, + max_version: '1.0.0-beta.1', + max_stable_version: null, + name: 'foo-bar', + newest_version: '1.0.0-beta.1', + repository: null, + updated_at: '2017-02-24T12:34:56Z', + versions: ['1'], + yanked: false, + }, + keywords: [], + versions: [ + { + id: '1', + crate: 'foo-bar', + crate_size: 0, + created_at: '2010-06-16T21:30:45Z', + dl_path: '/api/v1/crates/foo-bar/1.0.0-beta.1/download', + downloads: 0, + license: 'MIT/Apache-2.0', + links: { + dependencies: '/api/v1/crates/foo-bar/1.0.0-beta.1/dependencies', + version_downloads: '/api/v1/crates/foo-bar/1.0.0-beta.1/downloads', + }, + num: '1.0.0-beta.1', + published_by: null, + readme_path: '/api/v1/crates/foo-bar/1.0.0-beta.1/readme', + rust_version: null, + updated_at: '2017-02-24T12:34:56Z', + yanked: false, + yank_message: null, + }, + ], + }); + }); + test('includes related versions', async function (assert) { let crate = this.server.create('crate', { name: 'rand' }); this.server.create('version', { crate, num: '1.0.0' }); From fbe6234294cf19078511bf979e1e497677fc5d1b Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Fri, 10 Jan 2025 16:44:12 +0100 Subject: [PATCH 2/2] Fix non-canonical crate name pages Before the Ember Data 5 update it was possible to visit `/crates/foo-bar` when the crate was actually called `foo_bar` and vice-versa. The Ember Data 5 update broke this behavior, and this commit is restoring it, by using `queryRecord()` instead of `findRecord()`, which can no longer deal with requesting a certain `id` and getting a different one returned from the server. --- app/adapters/crate.js | 30 +++++++++++++++++++++++++----- app/routes/crate.js | 2 +- e2e/acceptance/crate.spec.ts | 16 +++++++++++++++- tests/acceptance/crate-test.js | 13 +++++++++++++ 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/app/adapters/crate.js b/app/adapters/crate.js index 544514604bf..5dc3b05f122 100644 --- a/app/adapters/crate.js +++ b/app/adapters/crate.js @@ -6,12 +6,23 @@ export default class CrateAdapter extends ApplicationAdapter { coalesceFindRequests = true; findRecord(store, type, id, snapshot) { - let { include } = snapshot; - // This ensures `crate.versions` are always fetched from another request. - if (include === undefined) { - snapshot.include = 'keywords,categories,downloads'; + return super.findRecord(store, type, id, setDefaultInclude(snapshot)); + } + + queryRecord(store, type, query, adapterOptions) { + return super.queryRecord(store, type, setDefaultInclude(query), adapterOptions); + } + + /** Removes the `name` query parameter and turns it into a path parameter instead */ + urlForQueryRecord(query) { + let baseUrl = super.urlForQueryRecord(...arguments); + if (!query.name) { + return baseUrl; } - return super.findRecord(store, type, id, snapshot); + + let crateName = query.name; + delete query.name; + return `${baseUrl}/${crateName}`; } groupRecordsForFindMany(store, snapshots) { @@ -22,3 +33,12 @@ export default class CrateAdapter extends ApplicationAdapter { return result; } } + +function setDefaultInclude(query) { + if (query.include === undefined) { + // This ensures `crate.versions` are always fetched from another request. + query.include = 'keywords,categories,downloads'; + } + + return query; +} diff --git a/app/routes/crate.js b/app/routes/crate.js index f8646db3baa..84a1fe56739 100644 --- a/app/routes/crate.js +++ b/app/routes/crate.js @@ -12,7 +12,7 @@ export default class CrateRoute extends Route { let crateName = params.crate_id; try { - return await this.store.findRecord('crate', crateName); + return this.store.peekRecord('crate', crateName) || (await this.store.queryRecord('crate', { name: crateName })); } catch (error) { if (error instanceof NotFoundError) { let title = `${crateName}: Crate not found`; diff --git a/e2e/acceptance/crate.spec.ts b/e2e/acceptance/crate.spec.ts index 906365cbc45..505545ae678 100644 --- a/e2e/acceptance/crate.spec.ts +++ b/e2e/acceptance/crate.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from '@/e2e/helper'; +import { expect, test } from '@/e2e/helper'; test.describe('Acceptance | crate page', { tag: '@acceptance' }, () => { test('visiting a crate page from the front page', async ({ page, mirage }) => { @@ -139,6 +139,20 @@ test.describe('Acceptance | crate page', { tag: '@acceptance' }, () => { await expect(page.locator('[data-test-try-again]')).toBeVisible(); }); + test('works for non-canonical names', async ({ page, mirage }) => { + await mirage.addHook(server => { + let crate = server.create('crate', { name: 'foo-bar' }); + server.create('version', { crate }); + }); + + await page.goto('/crates/foo_bar'); + + await expect(page).toHaveURL('/crates/foo_bar'); + await expect(page).toHaveTitle('foo-bar - crates.io: Rust Package Registry'); + + await expect(page.locator('[data-test-heading] [data-test-crate-name]')).toHaveText('foo-bar'); + }); + test('navigating to the all versions page', async ({ page, mirage }) => { await mirage.addHook(server => { server.loadFixtures(); diff --git a/tests/acceptance/crate-test.js b/tests/acceptance/crate-test.js index 0b607c54c22..ab6f22d53fe 100644 --- a/tests/acceptance/crate-test.js +++ b/tests/acceptance/crate-test.js @@ -131,6 +131,19 @@ module('Acceptance | crate page', function (hooks) { assert.dom('[data-test-try-again]').exists(); }); + test('works for non-canonical names', async function (assert) { + let crate = this.server.create('crate', { name: 'foo-bar' }); + this.server.create('version', { crate }); + + await visit('/crates/foo_bar'); + + assert.strictEqual(currentURL(), '/crates/foo_bar'); + assert.strictEqual(currentRouteName(), 'crate.index'); + assert.strictEqual(getPageTitle(), 'foo-bar - crates.io: Rust Package Registry'); + + assert.dom('[data-test-heading] [data-test-crate-name]').hasText('foo-bar'); + }); + test('navigating to the all versions page', async function (assert) { this.server.loadFixtures();