Merge pull request #10369 from Turbo87/canonical-names

Fix non-canonical crate name pages

Author: Turbo87

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;
+}

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`;

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();

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 ?? '';

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();
 

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' });