routes/crate: Get or fetch requested version by num

Previously, we fetched all versions and then found the requested version
within them. This commit changes the approach to either retrieve the
version from loaded versions or fetch it from the endpoint. This change
allows us to migrate to paginated versions in the future.

Author: eth3lbert

app/routes/crate/version-dependencies.js

@@ -1,25 +1,31 @@
+import { NotFoundError } from '@ember-data/adapter/error';
 import Route from '@ember/routing/route';
 import { inject as service } from '@ember/service';
 
 export default class VersionRoute extends Route {
+  @service store;
   @service router;
 
   async model(params, transition) {
     let crate = this.modelFor('crate');
 
-    let versions;
+    let requestedVersion = params.version_num;
+    let version;
     try {
-      versions = await crate.loadVersionsTask.perform();
+      version =
+        crate.loadedVersionsByNum.get(requestedVersion) ??
+        (await this.store.queryRecord('version', {
+          name: crate.id,
+          num: requestedVersion,
+        }));
     } catch (error) {
-      let title = `${crate.name}: Failed to load version data`;
-      return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
-    }
-
-    let requestedVersion = params.version_num;
-    let version = versions.find(version => version.num === requestedVersion);
-    if (!version) {
-      let title = `${crate.name}: Version ${requestedVersion} not found`;
-      return this.router.replaceWith('catch-all', { transition, title });
+      if (error instanceof NotFoundError) {
+        let title = `${crate.name}: Version ${requestedVersion} not found`;
+        return this.router.replaceWith('catch-all', { transition, title });
+      } else {
+        let title = `${crate.name}: Failed to load version data`;
+        return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
+      }
     }
 
     try {

app/routes/crate/version.js

@@ -1,44 +1,52 @@
+import { NotFoundError } from '@ember-data/adapter/error';
 import Route from '@ember/routing/route';
 import { inject as service } from '@ember/service';
 import { waitForPromise } from '@ember/test-waiters';
 
 import { didCancel } from 'ember-concurrency';
-import semverSort from 'semver/functions/rsort';
 
 import { AjaxError } from '../../utils/ajax';
 
 export default class VersionRoute extends Route {
   @service router;
   @service sentry;
+  @service store;
 
   async model(params, transition) {
     let crate = this.modelFor('crate');
 
-    let versions;
+    // TODO: Resolved version without waiting for versions to be resolved
+    // The main blocker for this right now is that we have a "belongsTo" relationship between
+    // `version-download` and `version` in sync mode. This requires us to wait for `versions` to
+    // exist before `version-download` can be created.
     try {
-      versions = await crate.loadVersionsTask.perform();
+      await crate.loadVersionsTask.perform();
     } catch (error) {
       let title = `${crate.name}: Failed to load version data`;
       return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
     }
 
     let version;
     let requestedVersion = params.version_num;
-    if (requestedVersion) {
-      version = versions.find(version => version.num === requestedVersion);
-      if (!version) {
-        let title = `${crate.name}: Version ${requestedVersion} not found`;
-        return this.router.replaceWith('catch-all', { transition, title });
-      }
-    } else {
-      let { default_version } = crate;
-      version = versions.find(version => version.num === default_version);
-
-      if (!version) {
-        let versionNums = versions.map(it => it.num);
-        semverSort(versionNums, { loose: true });
+    let num = requestedVersion || crate.default_version;
 
-        version = versions.find(version => version.num === versionNums[0]);
+    try {
+      version =
+        crate.loadedVersionsByNum.get(num) ??
+        (await crate.store.queryRecord('version', {
+          name: crate.id,
+          num,
+        }));
+    } catch (error) {
+      if (error instanceof NotFoundError) {
+        let title =
+          requestedVersion == null
+            ? `${crate.name}: Failed to find default version`
+            : `${crate.name}: Version ${requestedVersion} not found`;
+        return this.router.replaceWith('catch-all', { transition, title });
+      } else {
+        let title = `${crate.name}: Failed to load version data`;
+        return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
       }
     }
 

e2e/acceptance/crate-dependencies.spec.ts

@@ -62,20 +62,6 @@ test.describe('Acceptance | crate dependencies page', { tag: '@acceptance' }, ()
     await expect(page.locator('[data-test-try-again]')).toHaveCount(0);
   });
 
-  test('shows an error page if versions fail to load', async ({ page, msw, ember }) => {
-    let crate = msw.db.crate.create({ name: 'foo' });
-    msw.db.version.create({ crate, num: '2.0.0' });
-    await msw.worker.use(http.get('/api/v1/crates/:crate_name/versions', () => HttpResponse.json({}, { status: 500 })));
-
-    await page.goto('/crates/foo/1.0.0/dependencies');
-
-    await expect(page).toHaveURL('/crates/foo/1.0.0/dependencies');
-    await expect(page.locator('[data-test-404-page]')).toBeVisible();
-    await expect(page.locator('[data-test-title]')).toHaveText('foo: Failed to load version data');
-    await expect(page.locator('[data-test-go-back]')).toHaveCount(0);
-    await expect(page.locator('[data-test-try-again]')).toBeVisible();
-  });
-
   test('shows error message if loading of dependencies fails', async ({ page, msw }) => {
     let crate = msw.db.crate.create({ name: 'foo' });
     msw.db.version.create({ crate, num: '1.0.0' });

tests/acceptance/crate-dependencies-test.js

@@ -74,20 +74,6 @@ module('Acceptance | crate dependencies page', function (hooks) {
     assert.dom('[data-test-try-again]').doesNotExist();
   });
 
-  test('shows an error page if versions fail to load', async function (assert) {
-    let crate = this.db.crate.create({ name: 'foo' });
-    this.db.version.create({ crate, num: '2.0.0' });
-
-    this.worker.use(http.get('/api/v1/crates/:crate_name/versions', () => HttpResponse.json({}, { status: 500 })));
-
-    await visit('/crates/foo/1.0.0/dependencies');
-    assert.strictEqual(currentURL(), '/crates/foo/1.0.0/dependencies');
-    assert.dom('[data-test-404-page]').exists();
-    assert.dom('[data-test-title]').hasText('foo: Failed to load version data');
-    assert.dom('[data-test-go-back]').doesNotExist();
-    assert.dom('[data-test-try-again]').exists();
-  });
-
   test('shows error message if loading of dependencies fails', async function (assert) {
     let crate = this.db.crate.create({ name: 'foo' });
     this.db.version.create({ crate, num: '1.0.0' });