From e36d933d12ea486d5724449a19b0017e7f58531f Mon Sep 17 00:00:00 2001
From: Johannes Vollmer <32042925+johannesvollmer@users.noreply.github.com>
Date: Mon, 5 Feb 2024 12:05:47 +0100
Subject: [PATCH] [Crates] Only use non-yanked crate versions (ready for merge)
(#9949)
* prototype fix msrv using yanked versions
* make it prettier
* deduplicate schema definition and add version info helper function
* use newest version only if it wasnt yanked
* fix variable name typo
* remove unused import
* try add new test
* satisfy linter
* don't import "describe()"
* fixup
---------
Co-authored-by: chris48s <git@chris-shaw.dev>
---
services/crates/crates-base.js | 52 ++++++++++++---------
services/crates/crates-base.spec.js | 47 +++++++++++++++++++
services/crates/crates-downloads.service.js | 2 +-
services/crates/crates-downloads.tester.js | 4 +-
services/crates/crates-license.service.js | 4 +-
services/crates/crates-license.spec.js | 29 ++++++++----
services/crates/crates-msrv.service.js | 4 +-
services/crates/crates-version.service.js | 8 +---
services/crates/crates-version.spec.js | 11 -----
9 files changed, 106 insertions(+), 55 deletions(-)
create mode 100644 services/crates/crates-base.spec.js
delete mode 100644 services/crates/crates-version.spec.js
diff --git a/services/crates/crates-base.js b/services/crates/crates-base.js
index 28c1af3e7e..079f519de5 100644
--- a/services/crates/crates-base.js
+++ b/services/crates/crates-base.js
@@ -1,36 +1,27 @@
import Joi from 'joi'
import { nonNegativeInteger } from '../validators.js'
-import { BaseJsonService } from '../index.js'
+import { BaseJsonService, InvalidResponse } from '../index.js'
-const crateSchema = Joi.object({
+const versionSchema = Joi.object({
+ downloads: nonNegativeInteger,
+ num: Joi.string().required(),
+ license: Joi.string().required().allow(null),
+ rust_version: Joi.string().allow(null),
+})
+
+const crateResponseSchema = Joi.object({
crate: Joi.object({
downloads: nonNegativeInteger,
recent_downloads: nonNegativeInteger.allow(null),
max_version: Joi.string().required(),
}).required(),
- versions: Joi.array()
- .items(
- Joi.object({
- downloads: nonNegativeInteger,
- license: Joi.string().required().allow(null),
- rust_version: Joi.string().allow(null),
- }),
- )
- .min(1)
- .required(),
+ versions: Joi.array().items(versionSchema).min(1).required(),
}).required()
-const versionSchema = Joi.object({
- version: Joi.object({
- downloads: nonNegativeInteger,
- num: Joi.string().required(),
- license: Joi.string().required().allow(null),
- rust_version: Joi.string().allow(null),
- }).required(),
+const versionResponseSchema = Joi.object({
+ version: versionSchema.required(),
}).required()
-const schema = Joi.alternatives(crateSchema, versionSchema)
-
class BaseCratesService extends BaseJsonService {
static defaultBadgeData = { label: 'crates.io' }
@@ -38,8 +29,27 @@ class BaseCratesService extends BaseJsonService {
const url = version
? `https://crates.io/api/v1/crates/${crate}/${version}`
: `https://crates.io/api/v1/crates/${crate}?include=versions,downloads`
+ const schema = version ? versionResponseSchema : crateResponseSchema
return this._requestJson({ schema, url })
}
+
+ static getLatestVersion(response) {
+ return response.crate.max_stable_version
+ ? response.crate.max_stable_version
+ : response.crate.max_version
+ }
+
+ static getVersionObj(response) {
+ if (response.crate) {
+ const version = this.getLatestVersion(response)
+ const versionObj = response.versions.find(v => v.num === version)
+ if (versionObj === undefined) {
+ throw new InvalidResponse({ prettyMessage: 'version not found' })
+ }
+ return versionObj
+ }
+ return response.version
+ }
}
const description =
diff --git a/services/crates/crates-base.spec.js b/services/crates/crates-base.spec.js
new file mode 100644
index 0000000000..a68990acb1
--- /dev/null
+++ b/services/crates/crates-base.spec.js
@@ -0,0 +1,47 @@
+import { expect } from 'chai'
+import { test, given } from 'sazerac'
+import { BaseCratesService } from './crates-base.js'
+
+describe('BaseCratesService', function () {
+ describe('getLatestVersion', function () {
+ test(BaseCratesService.getLatestVersion, () => {
+ given({ crate: { max_version: '1.1.0' } }).expect('1.1.0')
+ given({
+ crate: { max_stable_version: '1.1.0', max_version: '1.9.0-alpha' },
+ }).expect('1.1.0')
+ })
+ })
+
+ describe('getVersionObj', function () {
+ const versions = [
+ /*
+ These versions are more recent, but we're going to ignore them
+ That might be because they were yanked, or because they're pre-releases.
+ */
+ { num: '1.3.0-beta' },
+ { num: '1.2.0' },
+ // this is the one we will select
+ { num: '1.1.0' },
+ ]
+
+ it('ignores more recent versions than max_stable_version', function () {
+ const response = {
+ crate: {
+ max_stable_version: '1.1.0',
+ },
+ versions,
+ }
+ expect(BaseCratesService.getVersionObj(response).num).to.equal('1.1.0')
+ })
+
+ it('ignores more recent versions than max_version', function () {
+ const response = {
+ crate: {
+ max_version: '1.1.0',
+ },
+ versions,
+ }
+ expect(BaseCratesService.getVersionObj(response).num).to.equal('1.1.0')
+ })
+ })
+})
diff --git a/services/crates/crates-downloads.service.js b/services/crates/crates-downloads.service.js
index f5435dbc90..ab5c0af5d9 100644
--- a/services/crates/crates-downloads.service.js
+++ b/services/crates/crates-downloads.service.js
@@ -73,7 +73,7 @@ export default class CratesDownloads extends BaseCratesService {
transform({ variant, json }) {
switch (variant) {
case 'dv':
- return json.crate ? json.versions[0].downloads : json.version.downloads
+ return this.constructor.getVersionObj(json).downloads
case 'dr':
return json.crate.recent_downloads || 0
default:
diff --git a/services/crates/crates-downloads.tester.js b/services/crates/crates-downloads.tester.js
index 24e7d1ca7d..8bc169570e 100644
--- a/services/crates/crates-downloads.tester.js
+++ b/services/crates/crates-downloads.tester.js
@@ -41,7 +41,9 @@ t.create('recent downloads (null)')
recent_downloads: null,
max_version: '0.2.71',
},
- versions: [{ downloads: 42, license: 'MIT OR Apache-2.0' }],
+ versions: [
+ { downloads: 42, license: 'MIT OR Apache-2.0', num: '0.2.71' },
+ ],
}),
)
.expectBadge({ label: 'recent downloads', message: '0' })
diff --git a/services/crates/crates-license.service.js b/services/crates/crates-license.service.js
index 42bffc2b17..9dc52e7ac0 100644
--- a/services/crates/crates-license.service.js
+++ b/services/crates/crates-license.service.js
@@ -36,8 +36,8 @@ export default class CratesLicense extends BaseCratesService {
static defaultBadgeData = { label: 'license', color: 'blue' }
- static transform({ version, versions }) {
- const license = version ? version.license : versions[0].license
+ static transform(response) {
+ const license = this.getVersionObj(response).license
if (!license) {
throw new InvalidResponse({ prettyMessage: 'invalid null license' })
}
diff --git a/services/crates/crates-license.spec.js b/services/crates/crates-license.spec.js
index 0e9db1858d..19424ab1d9 100644
--- a/services/crates/crates-license.spec.js
+++ b/services/crates/crates-license.spec.js
@@ -1,17 +1,21 @@
import { expect } from 'chai'
-import { test, given } from 'sazerac'
import { InvalidResponse } from '../index.js'
import CratesLicense from './crates-license.service.js'
describe('CratesLicense', function () {
- test(CratesLicense.transform, () => {
- given({
- version: { num: '1.0.0', license: 'MIT' },
- versions: [{ license: 'MIT/Apache 2.0' }],
- }).expect({ license: 'MIT' })
- given({
- versions: [{ license: 'MIT/Apache 2.0' }],
- }).expect({ license: 'MIT/Apache 2.0' })
+ it('extracts expected license given valid inputs', function () {
+ expect(
+ CratesLicense.transform({
+ version: { num: '1.0.0', license: 'MIT' },
+ }),
+ ).to.deep.equal({ license: 'MIT' })
+
+ expect(
+ CratesLicense.transform({
+ crate: { max_stable_version: '1.2.3' },
+ versions: [{ num: '1.2.3', license: 'MIT/Apache 2.0' }],
+ }),
+ ).to.deep.equal({ license: 'MIT/Apache 2.0' })
})
it('throws InvalidResponse on null license with specific version', function () {
@@ -23,7 +27,12 @@ describe('CratesLicense', function () {
})
it('throws InvalidResponse on null license with latest version', function () {
- expect(() => CratesLicense.transform({ versions: [{ license: null }] }))
+ expect(() =>
+ CratesLicense.transform({
+ crate: { max_stable_version: '1.2.3' },
+ versions: [{ num: '1.2.3', license: null }],
+ }),
+ )
.to.throw(InvalidResponse)
.with.property('prettyMessage', 'invalid null license')
})
diff --git a/services/crates/crates-msrv.service.js b/services/crates/crates-msrv.service.js
index 569745e557..415770f197 100644
--- a/services/crates/crates-msrv.service.js
+++ b/services/crates/crates-msrv.service.js
@@ -51,8 +51,8 @@ export default class CratesMSRV extends BaseCratesService {
static defaultBadgeData = { label: 'msrv', color: 'blue' }
- static transform({ version, versions }) {
- const msrv = version ? version.rust_version : versions[0].rust_version
+ static transform(response) {
+ const msrv = this.getVersionObj(response).rust_version
if (!msrv) {
throw new NotFound({ prettyMessage: 'unknown' })
}
diff --git a/services/crates/crates-version.service.js b/services/crates/crates-version.service.js
index fd85174d98..a618404f04 100644
--- a/services/crates/crates-version.service.js
+++ b/services/crates/crates-version.service.js
@@ -19,15 +19,9 @@ export default class CratesVersion extends BaseCratesService {
},
}
- transform(json) {
- return json.crate.max_stable_version
- ? json.crate.max_stable_version
- : json.crate.max_version
- }
-
async handle({ crate }) {
const json = await this.fetch({ crate })
- const version = this.transform(json)
+ const version = this.constructor.getLatestVersion(json)
return renderVersionBadge({ version })
}
}
diff --git a/services/crates/crates-version.spec.js b/services/crates/crates-version.spec.js
deleted file mode 100644
index f4d00f2d3d..0000000000
--- a/services/crates/crates-version.spec.js
+++ /dev/null
@@ -1,11 +0,0 @@
-import { test, given } from 'sazerac'
-import CratesVersion from './crates-version.service.js'
-
-describe('CratesVersion', function () {
- test(CratesVersion.prototype.transform, () => {
- given({ crate: { max_version: '1.1.0' } }).expect('1.1.0')
- given({
- crate: { max_stable_version: '1.1.0', max_version: '1.9.0-alpha' },
- }).expect('1.1.0')
- })
-})
--
GitLab