-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate response json in [apm appveyor cdnjs clojars gem] also affects [npm] #1883
Changes from 5 commits
07adf6a
8121a90
23890b3
99fc836
cf04796
ea0bac1
00f4176
01981e5
410b329
796e6e7
3cdf4a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,24 +6,27 @@ const { NotFound } = require('../errors') | |
const { addv: versionText } = require('../../lib/text-formatters') | ||
const { version: versionColor } = require('../../lib/color-formatters') | ||
|
||
const cdnjsSchema = Joi.object({ | ||
version: Joi.string(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you want There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On both CDNJS and Clojars the version key is intentionally optional because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment in the schema explaining why? |
||
}).required() | ||
|
||
module.exports = class Cdnjs extends BaseJsonService { | ||
async handle({ library }) { | ||
const url = `https://api.cdnjs.com/libraries/${library}?fields=version` | ||
const json = await this._requestJson({ | ||
url, | ||
schema: Joi.any(), | ||
schema: cdnjsSchema, | ||
}) | ||
|
||
if (Object.keys(json).length === 0) { | ||
/* Note the 'not found' response from cdnjs is: | ||
status code = 200, body = {} */ | ||
throw new NotFound() | ||
} | ||
const version = json.version || 0 | ||
|
||
return { | ||
message: versionText(version), | ||
color: versionColor(version), | ||
message: versionText(json.version), | ||
color: versionColor(json.version), | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,152 @@ | ||||
'use strict' | ||||
|
||||
const semver = require('semver') | ||||
const Joi = require('joi') | ||||
|
||||
const { BaseJsonService } = require('../base') | ||||
const { InvalidResponse } = require('../errors') | ||||
const { | ||||
downloadCount: downloadCountColor, | ||||
} = require('../../lib/color-formatters') | ||||
const { metric } = require('../../lib/text-formatters') | ||||
const { latest: latestVersion } = require('../../lib/version') | ||||
|
||||
const count = Joi.number() | ||||
.integer() | ||||
.min(0) | ||||
.required() | ||||
const downloadsSchema = Joi.alternatives().try( | ||||
Joi.object({ | ||||
downloads: count, | ||||
version_downloads: count, | ||||
}).required(), | ||||
Joi.array() | ||||
.items( | ||||
Joi.object({ | ||||
prerelease: Joi.boolean().required(), | ||||
number: Joi.string().required(), | ||||
downloads_count: count, | ||||
}) | ||||
) | ||||
.min(1) | ||||
.required() | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hit an edge case here. Depending on which downloads badge we are building ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, the npm downloads badge had the same issue. There I handled it with a switch:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes - that's a much better way of doing it |
||||
) | ||||
|
||||
module.exports = class GemDownloads extends BaseJsonService { | ||||
async fetch(repo, info) { | ||||
const endpoint = info === 'dv' ? 'versions/' : 'gems/' | ||||
const url = `https://rubygems.org/api/v1/${endpoint}${repo}.json` | ||||
return this._requestJson({ | ||||
url, | ||||
schema: downloadsSchema, | ||||
}) | ||||
} | ||||
|
||||
_getLabel(version, info) { | ||||
if (version) { | ||||
return 'downloads@' + version | ||||
} else { | ||||
if (info === 'dtv') { | ||||
return 'downloads@latest' | ||||
} else { | ||||
return 'downloads' | ||||
} | ||||
} | ||||
} | ||||
|
||||
async handle({ info, rubygem }) { | ||||
const splitRubygem = rubygem.split('/') | ||||
const repo = splitRubygem[0] | ||||
let version = | ||||
splitRubygem.length > 1 ? splitRubygem[splitRubygem.length - 1] : null | ||||
version = version === 'stable' ? version : semver.valid(version) | ||||
const label = this._getLabel(version, info) | ||||
const json = await this.fetch(repo, info) | ||||
|
||||
let downloads | ||||
if (info === 'dt') { | ||||
downloads = metric(json.downloads) | ||||
} else if (info === 'dtv') { | ||||
downloads = metric(json.version_downloads) | ||||
} else if (info === 'dv') { | ||||
let versionData | ||||
if (version !== null && version === 'stable') { | ||||
const versions = json | ||||
.filter(function(ver) { | ||||
return ver.prerelease === false | ||||
}) | ||||
.map(function(ver) { | ||||
return ver.number | ||||
}) | ||||
// Found latest stable version. | ||||
const stableVersion = latestVersion(versions) | ||||
versionData = json.filter(function(ver) { | ||||
return ver.number === stableVersion | ||||
})[0] | ||||
downloads = metric(versionData.downloads_count) | ||||
} else if (version !== null) { | ||||
versionData = json.filter(function(ver) { | ||||
return ver.number === version | ||||
})[0] | ||||
|
||||
downloads = metric(versionData.downloads_count) | ||||
} else { | ||||
throw new InvalidResponse({ | ||||
underlyingError: new Error('version is null'), | ||||
}) | ||||
} | ||||
} else { | ||||
throw new InvalidResponse({ | ||||
underlyingError: new Error('info is invalid'), | ||||
}) | ||||
} | ||||
|
||||
return { | ||||
label: label, | ||||
message: downloads, | ||||
color: downloadCountColor(downloads), | ||||
} | ||||
} | ||||
|
||||
// Metadata | ||||
static get defaultBadgeData() { | ||||
return { label: 'downloads' } | ||||
} | ||||
|
||||
static get category() { | ||||
return 'downloads' | ||||
} | ||||
|
||||
static get url() { | ||||
return { | ||||
base: 'gem', | ||||
format: '(dt|dtv|dv)/(.+)', | ||||
capture: ['info', 'rubygem'], | ||||
} | ||||
} | ||||
|
||||
static get examples() { | ||||
return [ | ||||
{ | ||||
title: 'Gem', | ||||
previewUrl: 'dv/rails/stable', | ||||
keywords: ['ruby'], | ||||
}, | ||||
{ | ||||
title: 'Gem', | ||||
previewUrl: 'dv/rails/4.1.0', | ||||
keywords: ['ruby'], | ||||
}, | ||||
{ | ||||
title: 'Gem', | ||||
previewUrl: 'dtv/rails', | ||||
keywords: ['ruby'], | ||||
}, | ||||
{ | ||||
title: 'Gem', | ||||
previewUrl: 'dt/rails', | ||||
keywords: ['ruby'], | ||||
}, | ||||
] | ||||
} | ||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
'use strict' | ||
|
||
const Joi = require('joi') | ||
|
||
const { BaseJsonService } = require('../base') | ||
const { floorCount: floorCountColor } = require('../../lib/color-formatters') | ||
|
||
module.exports = class GemOwner extends BaseJsonService { | ||
async handle({ user }) { | ||
const url = `https://rubygems.org/api/v1/owners/${user}/gems.json` | ||
const json = await this._requestJson({ | ||
url, | ||
schema: Joi.array(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this one is pending ^^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I intentionally left this because in this badge we are not using any of the data in the array - we are just counting the number of elements in it. As long the response is an array we're fine. I've moved the declaration out of the class and made it |
||
}) | ||
const count = json.length | ||
|
||
return { | ||
message: count, | ||
color: floorCountColor(count, 10, 50, 100), | ||
} | ||
} | ||
|
||
// Metadata | ||
static get defaultBadgeData() { | ||
return { label: 'gems' } | ||
} | ||
|
||
static get category() { | ||
return 'other' | ||
} | ||
|
||
static get url() { | ||
return { | ||
base: 'gem/u', | ||
format: '(.+)', | ||
capture: ['user'], | ||
} | ||
} | ||
|
||
static get examples() { | ||
return [ | ||
{ | ||
title: 'Gems', | ||
previewUrl: 'raphink', | ||
keywords: ['ruby'], | ||
}, | ||
] | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️