diff --git a/app/adapters/crate.js b/app/adapters/crate.js index 18b4badf87b..bac2eaac894 100644 --- a/app/adapters/crate.js +++ b/app/adapters/crate.js @@ -49,7 +49,7 @@ export default class CrateAdapter extends ApplicationAdapter { function setDefaultInclude(query) { if (query.include === undefined) { // This ensures `crate.versions` are always fetched from another request. - query.include = 'keywords,categories,downloads'; + query.include = 'keywords,categories,downloads,default_version'; } return query; diff --git a/app/adapters/version.js b/app/adapters/version.js index c8be99bdd27..bf2c4739868 100644 --- a/app/adapters/version.js +++ b/app/adapters/version.js @@ -6,4 +6,14 @@ export default class VersionAdapter extends ApplicationAdapter { let num = snapshot.record.num; return `/${this.namespace}/crates/${crateName}/${num}`; } + + urlForQueryRecord(query) { + let { name, num } = query ?? {}; + let baseUrl = this.buildURL('crate', name); + let url = `${baseUrl}/${num}`; + // The following used to remove them from URL's query string. + delete query.name; + delete query.num; + return url; + } } diff --git a/app/models/crate.js b/app/models/crate.js index cc73c860eb9..67b520a0c9e 100644 --- a/app/models/crate.js +++ b/app/models/crate.js @@ -67,6 +67,14 @@ export default class Crate extends Model { return Object.fromEntries(versions.slice().map(v => [v.id, v])); } + /** @return {Map} */ + @cached + get loadedVersionsByNum() { + let versionsRef = this.hasMany('versions'); + let values = versionsRef.value(); + return new Map(values?.map(ref => [ref.num, ref])); + } + @cached get releaseTrackSet() { let map = new Map(); let { versionsObj: versions, versionIdsBySemver } = this; diff --git a/app/routes/crate/dependencies.js b/app/routes/crate/dependencies.js index 8ffa3c3aa8a..68334f99636 100644 --- a/app/routes/crate/dependencies.js +++ b/app/routes/crate/dependencies.js @@ -6,11 +6,8 @@ export default class VersionRoute extends Route { async model() { let crate = this.modelFor('crate'); - let versions = await crate.loadVersionsTask.perform(); - let { default_version } = crate; - let version = versions.find(version => version.num === default_version) ?? versions.lastObject; - this.router.replaceWith('crate.version-dependencies', crate, version.num); + this.router.replaceWith('crate.version-dependencies', crate, default_version); } } diff --git a/app/routes/crate/version-dependencies.js b/app/routes/crate/version-dependencies.js index 75068fd7af4..9a85d43cafa 100644 --- a/app/routes/crate/version-dependencies.js +++ b/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 { diff --git a/app/routes/crate/version.js b/app/routes/crate/version.js index 683fbf8f3fd..a18450c7039 100644 --- a/app/routes/crate/version.js +++ b/app/routes/crate/version.js @@ -1,22 +1,26 @@ +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 }); @@ -24,21 +28,25 @@ export default class VersionRoute extends Route { 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 }); } } diff --git a/e2e/acceptance/crate-dependencies.spec.ts b/e2e/acceptance/crate-dependencies.spec.ts index ac792096dd0..401c2cb1673 100644 --- a/e2e/acceptance/crate-dependencies.spec.ts +++ b/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' }); diff --git a/tests/acceptance/crate-dependencies-test.js b/tests/acceptance/crate-dependencies-test.js index 29fb4ef6be9..a34139ab132 100644 --- a/tests/acceptance/crate-dependencies-test.js +++ b/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' }); diff --git a/tests/adapters/crate-test.js b/tests/adapters/crate-test.js index 86e84e97d13..2ad6571b231 100644 --- a/tests/adapters/crate-test.js +++ b/tests/adapters/crate-test.js @@ -29,18 +29,26 @@ module('Adapter | crate', function (hooks) { test('findRecord requests do not include versions by default', async function (assert) { let _foo = this.db.crate.create({ name: 'foo' }); - let version = this.db.version.create({ crate: _foo }); + this.db.version.create({ crate: _foo, num: '0.0.1' }); + this.db.version.create({ crate: _foo, num: '0.0.2' }); + this.db.version.create({ crate: _foo, num: '0.0.3' }); + + let versions = this.db.version.getAll().reverse(); + let default_version = versions.find(it => it.num === '0.0.3'); let store = this.owner.lookup('service:store'); let foo = await store.findRecord('crate', 'foo'); assert.strictEqual(foo?.name, 'foo'); - // versions should not be loaded yet + // Only `defaul_version` should be loaded let versionsRef = foo.hasMany('versions'); - assert.deepEqual(versionsRef.ids(), []); + assert.deepEqual(versionsRef.ids(), [`${default_version.id}`]); await versionsRef.load(); - assert.deepEqual(versionsRef.ids(), [`${version.id}`]); + assert.deepEqual( + versionsRef.ids(), + versions.map(it => `${it.id}`), + ); }); });